diff --git a/apps/server/src/routes/fs/routes/browse-project-files.ts b/apps/server/src/routes/fs/routes/browse-project-files.ts index 5e0ee22e..50afee0d 100644 --- a/apps/server/src/routes/fs/routes/browse-project-files.ts +++ b/apps/server/src/routes/fs/routes/browse-project-files.ts @@ -89,8 +89,13 @@ export function createBrowseProjectFilesHandler() { currentRelativePath = normalized; // Double-check the resolved path is within the project + // Use a separator-terminated prefix to prevent matching sibling dirs + // that share the same prefix (e.g. /projects/foo vs /projects/foobar). const resolvedTarget = path.resolve(targetPath); - if (!resolvedTarget.startsWith(resolvedProjectPath)) { + const projectPrefix = resolvedProjectPath.endsWith(path.sep) + ? resolvedProjectPath + : resolvedProjectPath + path.sep; + if (!resolvedTarget.startsWith(projectPrefix) && resolvedTarget !== resolvedProjectPath) { res.status(400).json({ success: false, error: 'Path traversal detected', @@ -130,7 +135,7 @@ export function createBrowseProjectFilesHandler() { }) .map((entry) => { const entryRelativePath = currentRelativePath - ? `${currentRelativePath}/${entry.name}` + ? path.posix.join(currentRelativePath.replace(/\\/g, '/'), entry.name) : entry.name; return { diff --git a/apps/server/src/routes/worktree/common.ts b/apps/server/src/routes/worktree/common.ts index 75c3a437..5f5c58f2 100644 --- a/apps/server/src/routes/worktree/common.ts +++ b/apps/server/src/routes/worktree/common.ts @@ -111,6 +111,17 @@ export function isValidBranchName(name: string): boolean { return /^[a-zA-Z0-9._\-/]+$/.test(name) && name.length < MAX_BRANCH_NAME_LENGTH; } +/** + * Validate git remote name to prevent command injection. + * Allowed characters: alphanumerics, hyphen, underscore, dot, and slash. + * Rejects empty strings and names that are too long. + */ +export function isValidRemoteName(name: string): boolean { + return ( + name.length > 0 && name.length < MAX_BRANCH_NAME_LENGTH && /^[a-zA-Z0-9._\-/]+$/.test(name) + ); +} + /** * Check if gh CLI is available on the system */ diff --git a/apps/server/src/routes/worktree/index.ts b/apps/server/src/routes/worktree/index.ts index 8acae439..4e812845 100644 --- a/apps/server/src/routes/worktree/index.ts +++ b/apps/server/src/routes/worktree/index.ts @@ -243,7 +243,7 @@ export function createWorktreeRoutes( '/cherry-pick', validatePathParams('worktreePath'), requireValidWorktree, - createCherryPickHandler() + createCherryPickHandler(events) ); // Generate PR description route @@ -259,7 +259,7 @@ export function createWorktreeRoutes( '/branch-commit-log', validatePathParams('worktreePath'), requireValidWorktree, - createBranchCommitLogHandler() + createBranchCommitLogHandler(events) ); return router; diff --git a/apps/server/src/routes/worktree/routes/branch-commit-log.ts b/apps/server/src/routes/worktree/routes/branch-commit-log.ts index ff7f2cd2..ce65fbf5 100644 --- a/apps/server/src/routes/worktree/routes/branch-commit-log.ts +++ b/apps/server/src/routes/worktree/routes/branch-commit-log.ts @@ -5,14 +5,19 @@ * any branch, not just the currently checked out one. Useful for cherry-pick workflows * where you need to browse commits from other branches. * + * The handler only validates input, invokes the service, streams lifecycle events + * via the EventEmitter, and sends the final JSON response. + * * Note: Git repository validation (isGitRepo, hasCommits) is handled by * the requireValidWorktree middleware in index.ts */ import type { Request, Response } from 'express'; -import { execGitCommand, getErrorMessage, logError } from '../common.js'; +import type { EventEmitter } from '../../../lib/events.js'; +import { getErrorMessage, logError } from '../common.js'; +import { getBranchCommitLog } from '../../../services/branch-commit-log-service.js'; -export function createBranchCommitLogHandler() { +export function createBranchCommitLogHandler(events: EventEmitter) { return async (req: Request, res: Response): Promise => { try { const { @@ -33,89 +38,40 @@ export function createBranchCommitLogHandler() { return; } - // Clamp limit to a reasonable range - const commitLimit = Math.min(Math.max(1, Number(limit) || 20), 100); + // Emit start event so the frontend can observe progress + events.emit('branchCommitLog:start', { + worktreePath, + branchName: branchName || 'HEAD', + limit, + }); - // Use the specified branch or default to HEAD - const targetRef = branchName || 'HEAD'; + // Delegate all Git work to the service + const result = await getBranchCommitLog(worktreePath, branchName, limit); - // Get detailed commit log for the specified branch - const logOutput = await execGitCommand( - [ - 'log', - targetRef, - `--max-count=${commitLimit}`, - '--format=%H%n%h%n%an%n%ae%n%aI%n%s%n%b%n---END---', - ], - worktreePath - ); + // Emit progress with the number of commits fetched + events.emit('branchCommitLog:progress', { + worktreePath, + branchName: result.branch, + commitsLoaded: result.total, + }); - // Parse the output into structured commit objects - const commits: Array<{ - hash: string; - shortHash: string; - author: string; - authorEmail: string; - date: string; - subject: string; - body: string; - files: string[]; - }> = []; - - const commitBlocks = logOutput.split('---END---\n').filter((block) => block.trim()); - - for (const block of commitBlocks) { - const lines = block.split('\n'); - if (lines.length >= 6) { - const hash = lines[0].trim(); - - // Get list of files changed in this commit - let files: string[] = []; - try { - const filesOutput = await execGitCommand( - ['diff-tree', '--no-commit-id', '--name-only', '-r', hash], - worktreePath - ); - files = filesOutput - .trim() - .split('\n') - .filter((f) => f.trim()); - } catch { - // Ignore errors getting file list - } - - commits.push({ - hash, - shortHash: lines[1].trim(), - author: lines[2].trim(), - authorEmail: lines[3].trim(), - date: lines[4].trim(), - subject: lines[5].trim(), - body: lines.slice(6).join('\n').trim(), - files, - }); - } - } - - // If branchName wasn't specified, get current branch for display - let displayBranch = branchName; - if (!displayBranch) { - const branchOutput = await execGitCommand( - ['rev-parse', '--abbrev-ref', 'HEAD'], - worktreePath - ); - displayBranch = branchOutput.trim(); - } + // Emit done event + events.emit('branchCommitLog:done', { + worktreePath, + branchName: result.branch, + total: result.total, + }); res.json({ success: true, - result: { - branch: displayBranch, - commits, - total: commits.length, - }, + result, }); } catch (error) { + // Emit error event so the frontend can react + events.emit('branchCommitLog:error', { + error: getErrorMessage(error), + }); + logError(error, 'Get branch commit log failed'); res.status(500).json({ success: false, error: getErrorMessage(error) }); } diff --git a/apps/server/src/routes/worktree/routes/checkout-branch.ts b/apps/server/src/routes/worktree/routes/checkout-branch.ts index 578db282..0eea6fc3 100644 --- a/apps/server/src/routes/worktree/routes/checkout-branch.ts +++ b/apps/server/src/routes/worktree/routes/checkout-branch.ts @@ -98,6 +98,19 @@ export function createCheckoutBranchHandler() { // Branch doesn't exist, good to create } + // If baseBranch is provided, verify it exists before using it + if (baseBranch) { + try { + await execGitCommand(['rev-parse', '--verify', baseBranch], resolvedPath); + } catch { + res.status(400).json({ + success: false, + error: `Base branch '${baseBranch}' does not exist`, + }); + return; + } + } + // Create and checkout the new branch (using argument array to avoid shell injection) // If baseBranch is provided, create the branch from that starting point const checkoutArgs = ['checkout', '-b', branchName]; diff --git a/apps/server/src/routes/worktree/routes/cherry-pick.ts b/apps/server/src/routes/worktree/routes/cherry-pick.ts index 828d3785..6b83ce61 100644 --- a/apps/server/src/routes/worktree/routes/cherry-pick.ts +++ b/apps/server/src/routes/worktree/routes/cherry-pick.ts @@ -4,17 +4,20 @@ * Applies commits from another branch onto the current branch. * Supports single or multiple commit cherry-picks. * + * Git business logic is delegated to cherry-pick-service.ts. + * Events are emitted at key lifecycle points for WebSocket subscribers. + * * Note: Git repository validation (isGitRepo, hasCommits) is handled by * the requireValidWorktree middleware in index.ts */ import type { Request, Response } from 'express'; -import { execGitCommand, getErrorMessage, logError } from '../common.js'; -import { createLogger } from '@automaker/utils'; +import path from 'path'; +import { getErrorMessage, logError } from '../common.js'; +import type { EventEmitter } from '../../../lib/events.js'; +import { verifyCommits, runCherryPick } from '../../../services/cherry-pick-service.js'; -const logger = createLogger('Worktree'); - -export function createCherryPickHandler() { +export function createCherryPickHandler(events: EventEmitter) { return async (req: Request, res: Response): Promise => { try { const { worktreePath, commitHashes, options } = req.body as { @@ -33,6 +36,9 @@ export function createCherryPickHandler() { return; } + // Normalize the path to prevent path traversal and ensure consistent paths + const resolvedWorktreePath = path.resolve(worktreePath); + if (!commitHashes || !Array.isArray(commitHashes) || commitHashes.length === 0) { res.status(400).json({ success: false, @@ -52,75 +58,64 @@ export function createCherryPickHandler() { } } - // Verify each commit exists - for (const hash of commitHashes) { - try { - await execGitCommand(['rev-parse', '--verify', hash], worktreePath); - } catch { - res.status(400).json({ - success: false, - error: `Commit "${hash}" does not exist`, - }); - return; - } + // Verify each commit exists via the service + const invalidHash = await verifyCommits(resolvedWorktreePath, commitHashes); + if (invalidHash !== null) { + res.status(400).json({ + success: false, + error: `Commit "${invalidHash}" does not exist`, + }); + return; } - // Build cherry-pick command args - const args = ['cherry-pick']; - if (options?.noCommit) { - args.push('--no-commit'); - } - // Add commit hashes in order - args.push(...commitHashes); + // Emit started event + events.emit('cherry-pick:started', { + worktreePath: resolvedWorktreePath, + commitHashes, + options, + }); - // Execute the cherry-pick - try { - await execGitCommand(args, worktreePath); + // Execute the cherry-pick via the service + const result = await runCherryPick(resolvedWorktreePath, commitHashes, options); - // Get current branch name - const branchOutput = await execGitCommand( - ['rev-parse', '--abbrev-ref', 'HEAD'], - worktreePath - ); + if (result.success) { + // Emit success event + events.emit('cherry-pick:success', { + worktreePath: resolvedWorktreePath, + commitHashes, + branch: result.branch, + }); res.json({ success: true, result: { - cherryPicked: true, - commitHashes, - branch: branchOutput.trim(), - message: `Successfully cherry-picked ${commitHashes.length} commit(s)`, + cherryPicked: result.cherryPicked, + commitHashes: result.commitHashes, + branch: result.branch, + message: result.message, }, }); - } catch (cherryPickError: unknown) { - // Check if this is a cherry-pick conflict - const err = cherryPickError as { stdout?: string; stderr?: string; message?: string }; - const output = `${err.stdout || ''} ${err.stderr || ''} ${err.message || ''}`; - const hasConflicts = - output.includes('CONFLICT') || - output.includes('cherry-pick failed') || - output.includes('could not apply'); + } else if (result.hasConflicts) { + // Emit conflict event + events.emit('cherry-pick:conflict', { + worktreePath: resolvedWorktreePath, + commitHashes, + aborted: result.aborted, + }); - if (hasConflicts) { - // Abort the cherry-pick to leave the repo in a clean state - try { - await execGitCommand(['cherry-pick', '--abort'], worktreePath); - } catch { - logger.warn('Failed to abort cherry-pick after conflict'); - } - - res.status(409).json({ - success: false, - error: `Cherry-pick CONFLICT: Could not apply commit(s) cleanly. Conflicts need to be resolved manually.`, - hasConflicts: true, - }); - return; - } - - // Re-throw non-conflict errors - throw cherryPickError; + res.status(409).json({ + success: false, + error: result.error, + hasConflicts: true, + aborted: result.aborted, + }); } } catch (error) { + // Emit failure event + events.emit('cherry-pick:failure', { + error: getErrorMessage(error), + }); + logError(error, 'Cherry-pick failed'); res.status(500).json({ success: false, error: getErrorMessage(error) }); } diff --git a/apps/server/src/routes/worktree/routes/commit-log.ts b/apps/server/src/routes/worktree/routes/commit-log.ts index 85207b53..1fda67b7 100644 --- a/apps/server/src/routes/worktree/routes/commit-log.ts +++ b/apps/server/src/routes/worktree/routes/commit-log.ts @@ -45,7 +45,7 @@ export function createCommitLogHandler() { files: string[]; }> = []; - const commitBlocks = logOutput.split('---END---\n').filter((block) => block.trim()); + const commitBlocks = logOutput.split('---END---').filter((block) => block.trim()); for (const block of commitBlocks) { const lines = block.split('\n'); diff --git a/apps/server/src/routes/worktree/routes/create-pr.ts b/apps/server/src/routes/worktree/routes/create-pr.ts index e2d0886f..0ef1a7e8 100644 --- a/apps/server/src/routes/worktree/routes/create-pr.ts +++ b/apps/server/src/routes/worktree/routes/create-pr.ts @@ -8,9 +8,12 @@ import { logError, execAsync, execEnv, + execGitCommand, isValidBranchName, + isValidRemoteName, isGhCliAvailable, } from '../common.js'; +import { spawnProcess } from '@automaker/platform'; import { updateWorktreePRInfo } from '../../../lib/worktree-metadata.js'; import { createLogger } from '@automaker/utils'; import { validatePRState } from '@automaker/types'; @@ -91,12 +94,9 @@ export function createCreatePRHandler() { logger.debug(`Running: git add -A`); await execAsync('git add -A', { cwd: worktreePath, env: execEnv }); - // Create commit + // Create commit — pass message as a separate arg to avoid shell injection logger.debug(`Running: git commit`); - await execAsync(`git commit -m "${message.replace(/"/g, '\\"')}"`, { - cwd: worktreePath, - env: execEnv, - }); + await execGitCommand(['commit', '-m', message], worktreePath); // Get commit hash const { stdout: hashOutput } = await execAsync('git rev-parse HEAD', { @@ -119,11 +119,20 @@ export function createCreatePRHandler() { } } + // Validate remote name before use to prevent command injection + if (remote !== undefined && !isValidRemoteName(remote)) { + res.status(400).json({ + success: false, + error: 'Invalid remote name contains unsafe characters', + }); + return; + } + // Push the branch to remote (use selected remote or default to 'origin') const pushRemote = remote || 'origin'; let pushError: string | null = null; try { - await execAsync(`git push -u ${pushRemote} ${branchName}`, { + await execAsync(`git push ${pushRemote} ${branchName}`, { cwd: worktreePath, env: execEnv, }); @@ -301,27 +310,35 @@ export function createCreatePRHandler() { // Only create a new PR if one doesn't already exist if (!prUrl) { try { - // Build gh pr create command - let prCmd = `gh pr create --base "${base}"`; + // Build gh pr create args as an array to avoid shell injection on + // title/body (backticks, $, \ were unsafe with string interpolation) + const prArgs = ['pr', 'create', '--base', base]; // If this is a fork (has upstream remote), specify the repo and head if (upstreamRepo && originOwner) { // For forks: --repo specifies where to create PR, --head specifies source - prCmd += ` --repo "${upstreamRepo}" --head "${originOwner}:${branchName}"`; + prArgs.push('--repo', upstreamRepo, '--head', `${originOwner}:${branchName}`); } else { // Not a fork, just specify the head branch - prCmd += ` --head "${branchName}"`; + prArgs.push('--head', branchName); } - prCmd += ` --title "${title.replace(/"/g, '\\"')}" --body "${body.replace(/"/g, '\\"')}" ${draftFlag}`; - prCmd = prCmd.trim(); + prArgs.push('--title', title, '--body', body); + if (draft) prArgs.push('--draft'); - logger.debug(`Creating PR with command: ${prCmd}`); - const { stdout: prOutput } = await execAsync(prCmd, { + logger.debug(`Creating PR with args: gh ${prArgs.join(' ')}`); + const prResult = await spawnProcess({ + command: 'gh', + args: prArgs, cwd: worktreePath, env: execEnv, }); - prUrl = prOutput.trim(); + if (prResult.exitCode !== 0) { + throw Object.assign(new Error(prResult.stderr || 'gh pr create failed'), { + stderr: prResult.stderr, + }); + } + prUrl = prResult.stdout.trim(); logger.info(`PR created: ${prUrl}`); // Extract PR number and store metadata for newly created PR diff --git a/apps/server/src/routes/worktree/routes/create.ts b/apps/server/src/routes/worktree/routes/create.ts index 1af08850..fce51401 100644 --- a/apps/server/src/routes/worktree/routes/create.ts +++ b/apps/server/src/routes/worktree/routes/create.ts @@ -11,10 +11,10 @@ import type { Request, Response } from 'express'; import { exec } from 'child_process'; import { promisify } from 'util'; import path from 'path'; -import fs from 'fs/promises'; import * as secureFs from '../../../lib/secure-fs.js'; import type { EventEmitter } from '../../../lib/events.js'; import type { SettingsService } from '../../../services/settings-service.js'; +import { WorktreeService } from '../../../services/worktree-service.js'; import { isGitRepo } from '@automaker/git-utils'; import { getErrorMessage, @@ -83,66 +83,9 @@ async function findExistingWorktreeForBranch( } } -/** - * Copy configured files from project root into the new worktree. - * Reads worktreeCopyFiles from project settings and copies each file/directory. - * Silently skips files that don't exist in the source. - */ -async function copyConfiguredFiles( - projectPath: string, - worktreePath: string, - settingsService?: SettingsService -): Promise { - if (!settingsService) return; - - try { - const projectSettings = await settingsService.getProjectSettings(projectPath); - const copyFiles = projectSettings.worktreeCopyFiles; - - if (!copyFiles || copyFiles.length === 0) return; - - for (const relativePath of copyFiles) { - // Security: prevent path traversal - const normalized = path.normalize(relativePath); - if (normalized.startsWith('..') || path.isAbsolute(normalized)) { - logger.warn(`Skipping suspicious copy path: ${relativePath}`); - continue; - } - - const sourcePath = path.join(projectPath, normalized); - const destPath = path.join(worktreePath, normalized); - - try { - // Check if source exists - const stat = await fs.stat(sourcePath); - - // Ensure destination directory exists - const destDir = path.dirname(destPath); - await fs.mkdir(destDir, { recursive: true }); - - if (stat.isDirectory()) { - // Recursively copy directory - await fs.cp(sourcePath, destPath, { recursive: true, force: true }); - logger.info(`Copied directory "${normalized}" to worktree`); - } else { - // Copy single file - await fs.copyFile(sourcePath, destPath); - logger.info(`Copied file "${normalized}" to worktree`); - } - } catch (err) { - if ((err as NodeJS.ErrnoException).code === 'ENOENT') { - logger.debug(`Skipping copy of "${normalized}" - file not found in project root`); - } else { - logger.warn(`Failed to copy "${normalized}" to worktree:`, err); - } - } - } - } catch (error) { - logger.warn('Failed to read project settings for file copying:', error); - } -} - export function createCreateHandler(events: EventEmitter, settingsService?: SettingsService) { + const worktreeService = new WorktreeService(); + return async (req: Request, res: Response): Promise => { try { const { projectPath, branchName, baseBranch } = req.body as { @@ -263,7 +206,17 @@ export function createCreateHandler(events: EventEmitter, settingsService?: Sett // Copy configured files into the new worktree before responding // This runs synchronously to ensure files are in place before any init script - await copyConfiguredFiles(projectPath, absoluteWorktreePath, settingsService); + try { + await worktreeService.copyConfiguredFiles( + projectPath, + absoluteWorktreePath, + settingsService, + events + ); + } catch (copyErr) { + // Log but don't fail worktree creation – files may be partially copied + logger.warn('Some configured files failed to copy to worktree:', copyErr); + } // Respond immediately (non-blocking) res.json({ diff --git a/apps/server/src/routes/worktree/routes/generate-pr-description.ts b/apps/server/src/routes/worktree/routes/generate-pr-description.ts index 10f9e147..0f272e71 100644 --- a/apps/server/src/routes/worktree/routes/generate-pr-description.ts +++ b/apps/server/src/routes/worktree/routes/generate-pr-description.ts @@ -7,7 +7,7 @@ */ import type { Request, Response } from 'express'; -import { exec } from 'child_process'; +import { execFile } from 'child_process'; import { promisify } from 'util'; import { existsSync } from 'fs'; import { join } from 'path'; @@ -20,7 +20,7 @@ import { getErrorMessage, logError } from '../common.js'; import { getPhaseModelWithOverrides } from '../../../lib/settings-helpers.js'; const logger = createLogger('GeneratePRDescription'); -const execAsync = promisify(exec); +const execFileAsync = promisify(execFile); /** Timeout for AI provider calls in milliseconds (30 seconds) */ const AI_TIMEOUT_MS = 30_000; @@ -59,20 +59,33 @@ async function* withTimeout( generator: AsyncIterable, timeoutMs: number ): AsyncGenerator { + let timerId: ReturnType | undefined; + const timeoutPromise = new Promise((_, reject) => { - setTimeout(() => reject(new Error(`AI provider timed out after ${timeoutMs}ms`)), timeoutMs); + timerId = setTimeout( + () => reject(new Error(`AI provider timed out after ${timeoutMs}ms`)), + timeoutMs + ); }); const iterator = generator[Symbol.asyncIterator](); let done = false; - while (!done) { - const result = await Promise.race([iterator.next(), timeoutPromise]); - if (result.done) { - done = true; - } else { - yield result.value; + try { + while (!done) { + const result = await Promise.race([iterator.next(), timeoutPromise]).catch(async (err) => { + // Timeout (or other error) — attempt to gracefully close the source generator + await iterator.return?.(); + throw err; + }); + if (result.done) { + done = true; + } else { + yield result.value; + } } + } finally { + clearTimeout(timerId); } } @@ -129,12 +142,24 @@ export function createGeneratePRDescriptionHandler( return; } + // Validate baseBranch to allow only safe branch name characters + if (baseBranch !== undefined && !/^[\w.\-/]+$/.test(baseBranch)) { + const response: GeneratePRDescriptionErrorResponse = { + success: false, + error: 'baseBranch contains invalid characters', + }; + res.status(400).json(response); + return; + } + logger.info(`Generating PR description for worktree: ${worktreePath}`); // Get current branch name - const { stdout: branchOutput } = await execAsync('git rev-parse --abbrev-ref HEAD', { - cwd: worktreePath, - }); + const { stdout: branchOutput } = await execFileAsync( + 'git', + ['rev-parse', '--abbrev-ref', 'HEAD'], + { cwd: worktreePath } + ); const branchName = branchOutput.trim(); // Determine the base branch for comparison @@ -149,7 +174,7 @@ export function createGeneratePRDescriptionHandler( let diffIncludesUncommitted = false; try { // First, try to get diff against the base branch - const { stdout: branchDiff } = await execAsync(`git diff ${base}...HEAD`, { + const { stdout: branchDiff } = await execFileAsync('git', ['diff', `${base}...HEAD`], { cwd: worktreePath, maxBuffer: 1024 * 1024 * 5, // 5MB buffer }); @@ -160,17 +185,21 @@ export function createGeneratePRDescriptionHandler( // If branch comparison fails (e.g., base branch doesn't exist locally), // try fetching and comparing against remote base try { - const { stdout: remoteDiff } = await execAsync(`git diff origin/${base}...HEAD`, { - cwd: worktreePath, - maxBuffer: 1024 * 1024 * 5, - }); + const { stdout: remoteDiff } = await execFileAsync( + 'git', + ['diff', `origin/${base}...HEAD`], + { + cwd: worktreePath, + maxBuffer: 1024 * 1024 * 5, + } + ); diff = remoteDiff; // git diff origin/base...HEAD only shows committed changes diffIncludesUncommitted = false; } catch { // Fall back to getting all uncommitted + committed changes try { - const { stdout: allDiff } = await execAsync('git diff HEAD', { + const { stdout: allDiff } = await execFileAsync('git', ['diff', 'HEAD'], { cwd: worktreePath, maxBuffer: 1024 * 1024 * 5, }); @@ -179,11 +208,11 @@ export function createGeneratePRDescriptionHandler( diffIncludesUncommitted = true; } catch { // Last resort: get staged + unstaged changes - const { stdout: stagedDiff } = await execAsync('git diff --cached', { + const { stdout: stagedDiff } = await execFileAsync('git', ['diff', '--cached'], { cwd: worktreePath, maxBuffer: 1024 * 1024 * 5, }); - const { stdout: unstagedDiff } = await execAsync('git diff', { + const { stdout: unstagedDiff } = await execFileAsync('git', ['diff'], { cwd: worktreePath, maxBuffer: 1024 * 1024 * 5, }); @@ -200,7 +229,7 @@ export function createGeneratePRDescriptionHandler( // when the primary diff method (base...HEAD) was used, since it only shows committed changes. let hasUncommittedChanges = false; try { - const { stdout: statusOutput } = await execAsync('git status --porcelain', { + const { stdout: statusOutput } = await execFileAsync('git', ['status', '--porcelain'], { cwd: worktreePath, }); hasUncommittedChanges = statusOutput.trim().length > 0; @@ -212,7 +241,7 @@ export function createGeneratePRDescriptionHandler( // Get staged changes try { - const { stdout: stagedDiff } = await execAsync('git diff --cached', { + const { stdout: stagedDiff } = await execFileAsync('git', ['diff', '--cached'], { cwd: worktreePath, maxBuffer: 1024 * 1024 * 5, }); @@ -225,7 +254,7 @@ export function createGeneratePRDescriptionHandler( // Get unstaged changes (tracked files only) try { - const { stdout: unstagedDiff } = await execAsync('git diff', { + const { stdout: unstagedDiff } = await execFileAsync('git', ['diff'], { cwd: worktreePath, maxBuffer: 1024 * 1024 * 5, }); @@ -259,8 +288,9 @@ export function createGeneratePRDescriptionHandler( // Also get the commit log for context let commitLog = ''; try { - const { stdout: logOutput } = await execAsync( - `git log ${base}..HEAD --oneline --no-decorate 2>/dev/null || git log --oneline -10 --no-decorate`, + const { stdout: logOutput } = await execFileAsync( + 'git', + ['log', `${base}..HEAD`, '--oneline', '--no-decorate'], { cwd: worktreePath, maxBuffer: 1024 * 1024, @@ -268,7 +298,20 @@ export function createGeneratePRDescriptionHandler( ); commitLog = logOutput.trim(); } catch { - // Ignore commit log errors + // If comparing against base fails, fall back to recent commits + try { + const { stdout: logOutput } = await execFileAsync( + 'git', + ['log', '--oneline', '-10', '--no-decorate'], + { + cwd: worktreePath, + maxBuffer: 1024 * 1024, + } + ); + commitLog = logOutput.trim(); + } catch { + // Ignore commit log errors + } } if (!diff.trim() && !commitLog.trim()) { diff --git a/apps/server/src/routes/worktree/routes/stash-apply.ts b/apps/server/src/routes/worktree/routes/stash-apply.ts index 12c80306..7965e481 100644 --- a/apps/server/src/routes/worktree/routes/stash-apply.ts +++ b/apps/server/src/routes/worktree/routes/stash-apply.ts @@ -40,7 +40,17 @@ export function createStashApplyHandler() { return; } - const stashRef = `stash@{${stashIndex}}`; + const idx = typeof stashIndex === 'string' ? Number(stashIndex) : stashIndex; + + if (!Number.isInteger(idx) || idx < 0) { + res.status(400).json({ + success: false, + error: 'stashIndex must be a non-negative integer', + }); + return; + } + + const stashRef = `stash@{${idx}}`; const operation = pop ? 'pop' : 'apply'; try { diff --git a/apps/server/src/routes/worktree/routes/stash-drop.ts b/apps/server/src/routes/worktree/routes/stash-drop.ts index 235ade91..9df0f046 100644 --- a/apps/server/src/routes/worktree/routes/stash-drop.ts +++ b/apps/server/src/routes/worktree/routes/stash-drop.ts @@ -30,7 +30,7 @@ export function createStashDropHandler() { return; } - if (stashIndex === undefined || stashIndex === null) { + if (!Number.isInteger(stashIndex) || stashIndex < 0) { res.status(400).json({ success: false, error: 'stashIndex required', diff --git a/apps/server/src/routes/worktree/routes/stash-list.ts b/apps/server/src/routes/worktree/routes/stash-list.ts index 096dd25d..a44c0f40 100644 --- a/apps/server/src/routes/worktree/routes/stash-list.ts +++ b/apps/server/src/routes/worktree/routes/stash-list.ts @@ -71,9 +71,10 @@ export function createStashListHandler() { const message = parts[1].trim(); const date = parts[2].trim(); - // Extract index from stash@{N} + // Extract index from stash@{N}; skip entries that don't match the expected format const indexMatch = refSpec.match(/stash@\{(\d+)\}/); - const index = indexMatch ? parseInt(indexMatch[1], 10) : 0; + if (!indexMatch) continue; + const index = parseInt(indexMatch[1], 10); // Extract branch name from message (format: "WIP on branch: hash message" or "On branch: hash message") let branch = ''; diff --git a/apps/server/src/routes/worktree/routes/switch-branch.ts b/apps/server/src/routes/worktree/routes/switch-branch.ts index 9fe25eda..3e413844 100644 --- a/apps/server/src/routes/worktree/routes/switch-branch.ts +++ b/apps/server/src/routes/worktree/routes/switch-branch.ts @@ -197,13 +197,7 @@ export function createSwitchBranchHandler() { isRemote = true; const parsed = parseRemoteBranch(branchName); if (parsed) { - // If a local branch with the same name already exists, just switch to it - if (await localBranchExists(worktreePath, parsed.branch)) { - targetBranch = parsed.branch; - } else { - // Will create a local tracking branch from the remote - targetBranch = parsed.branch; - } + targetBranch = parsed.branch; } } @@ -307,11 +301,37 @@ export function createSwitchBranchHandler() { } catch (checkoutError) { // If checkout failed and we stashed, try to restore the stash if (didStash) { - try { - await popStash(worktreePath); - } catch { - // Ignore errors restoring stash - it's still in the stash list + const popResult = await popStash(worktreePath); + if (popResult.hasConflicts) { + // Stash pop itself produced merge conflicts — the working tree is now in a + // conflicted state even though the checkout failed. Surface this clearly so + // the caller can prompt the user (or AI) to resolve conflicts rather than + // simply retrying the branch switch. + const checkoutErrorMsg = getErrorMessage(checkoutError); + res.status(500).json({ + success: false, + error: checkoutErrorMsg, + stashPopConflicts: true, + stashPopConflictMessage: + 'Stash pop resulted in conflicts: your stashed changes were partially reapplied ' + + 'but produced merge conflicts. Please resolve the conflicts before retrying the branch switch.', + }); + return; + } else if (!popResult.success) { + // Stash pop failed for a non-conflict reason; the stash entry is still intact. + // Include this detail alongside the original checkout error. + const checkoutErrorMsg = getErrorMessage(checkoutError); + const combinedMessage = + `${checkoutErrorMsg}. Additionally, restoring your stashed changes failed: ` + + `${popResult.error ?? 'unknown error'} — your changes are still saved in the stash.`; + res.status(500).json({ + success: false, + error: combinedMessage, + stashPopConflicts: false, + }); + return; } + // popResult.success === true: stash was cleanly restored, re-throw the checkout error } throw checkoutError; } diff --git a/apps/server/src/services/auto-loop-coordinator.ts b/apps/server/src/services/auto-loop-coordinator.ts index cc307ede..29c36558 100644 --- a/apps/server/src/services/auto-loop-coordinator.ts +++ b/apps/server/src/services/auto-loop-coordinator.ts @@ -185,14 +185,17 @@ export class AutoLoopCoordinator { // Load all features for dependency checking (if callback provided) const allFeatures = this.loadAllFeaturesFn ? await this.loadAllFeaturesFn(projectPath) - : pendingFeatures; + : undefined; - // Filter to eligible features: not running, not finished, and dependencies satisfied + // Filter to eligible features: not running, not finished, and dependencies satisfied. + // When loadAllFeaturesFn is not provided, allFeatures is undefined and we bypass + // dependency checks (returning true) to avoid false negatives caused by completed + // features being absent from pendingFeatures. const eligibleFeatures = pendingFeatures.filter( (f) => !this.isFeatureRunningFn(f.id) && !this.isFeatureFinishedFn(f) && - areDependenciesSatisfied(f, allFeatures) + (this.loadAllFeaturesFn ? areDependenciesSatisfied(f, allFeatures!) : true) ); // Sort eligible features by priority (lower number = higher priority, default 2) @@ -412,10 +415,12 @@ export class AutoLoopCoordinator { const projectId = settings.projects?.find((p) => p.path === projectPath)?.id; const autoModeByWorktree = settings.autoModeByWorktree; if (projectId && autoModeByWorktree && typeof autoModeByWorktree === 'object') { - // branchName is already normalized to null for the primary branch by callers - // (e.g., checkWorktreeCapacity, startAutoLoopForProject), so we only - // need to convert null to '__main__' for the worktree key lookup - const normalizedBranch = branchName === null ? '__main__' : branchName; + // Normalize both null and 'main' to '__main__' to match the same + // canonicalization used by getWorktreeAutoLoopKey, ensuring that + // lookups for the primary branch always use the '__main__' sentinel + // regardless of whether the caller passed null or the string 'main'. + const normalizedBranch = + branchName === null || branchName === 'main' ? '__main__' : branchName; const worktreeId = `${projectId}::${normalizedBranch}`; if ( worktreeId in autoModeByWorktree && diff --git a/apps/server/src/services/branch-commit-log-service.ts b/apps/server/src/services/branch-commit-log-service.ts new file mode 100644 index 00000000..2e42a1c9 --- /dev/null +++ b/apps/server/src/services/branch-commit-log-service.ts @@ -0,0 +1,119 @@ +/** + * Service for fetching branch commit log data. + * + * Extracts the heavy Git command execution and parsing logic from the + * branch-commit-log route handler so the handler only validates input, + * invokes this service, streams lifecycle events, and sends the response. + */ + +import { execGitCommand } from '../routes/worktree/common.js'; + +// ============================================================================ +// Types +// ============================================================================ + +export interface BranchCommit { + hash: string; + shortHash: string; + author: string; + authorEmail: string; + date: string; + subject: string; + body: string; + files: string[]; +} + +export interface BranchCommitLogResult { + branch: string; + commits: BranchCommit[]; + total: number; +} + +// ============================================================================ +// Service +// ============================================================================ + +/** + * Fetch the commit log for a specific branch (or HEAD). + * + * Runs `git log`, `git diff-tree`, and `git rev-parse` inside + * the given worktree path and returns a structured result. + * + * @param worktreePath - Absolute path to the worktree / repository + * @param branchName - Branch to query (omit or pass undefined for HEAD) + * @param limit - Maximum number of commits to return (clamped 1-100) + */ +export async function getBranchCommitLog( + worktreePath: string, + branchName: string | undefined, + limit: number +): Promise { + // Clamp limit to a reasonable range + const parsedLimit = Number(limit); + const commitLimit = Math.min(Math.max(1, Number.isFinite(parsedLimit) ? parsedLimit : 20), 100); + + // Use the specified branch or default to HEAD + const targetRef = branchName || 'HEAD'; + + // Get detailed commit log for the specified branch + const logOutput = await execGitCommand( + [ + 'log', + targetRef, + `--max-count=${commitLimit}`, + '--format=%H%n%h%n%an%n%ae%n%aI%n%s%n%b%n---END---', + ], + worktreePath + ); + + // Parse the output into structured commit objects + const commits: BranchCommit[] = []; + + const commitBlocks = logOutput.split('---END---\n').filter((block) => block.trim()); + + for (const block of commitBlocks) { + const lines = block.split('\n'); + if (lines.length >= 6) { + const hash = lines[0].trim(); + + // Get list of files changed in this commit + let files: string[] = []; + try { + const filesOutput = await execGitCommand( + ['diff-tree', '--no-commit-id', '--name-only', '-r', hash], + worktreePath + ); + files = filesOutput + .trim() + .split('\n') + .filter((f) => f.trim()); + } catch { + // Ignore errors getting file list + } + + commits.push({ + hash, + shortHash: lines[1].trim(), + author: lines[2].trim(), + authorEmail: lines[3].trim(), + date: lines[4].trim(), + subject: lines[5].trim(), + body: lines.slice(6).join('\n').trim(), + files, + }); + } + } + + // If branchName wasn't specified, get current branch for display + let displayBranch = branchName; + if (!displayBranch) { + const branchOutput = await execGitCommand(['rev-parse', '--abbrev-ref', 'HEAD'], worktreePath); + displayBranch = branchOutput.trim(); + } + + return { + branch: displayBranch, + commits, + total: commits.length, + }; +} diff --git a/apps/server/src/services/cherry-pick-service.ts b/apps/server/src/services/cherry-pick-service.ts new file mode 100644 index 00000000..672833cb --- /dev/null +++ b/apps/server/src/services/cherry-pick-service.ts @@ -0,0 +1,162 @@ +/** + * CherryPickService - Cherry-pick git operations without HTTP + * + * Extracted from worktree cherry-pick route to encapsulate all git + * cherry-pick business logic in a single service. Follows the same + * pattern as merge-service.ts. + */ + +import { createLogger } from '@automaker/utils'; +import { spawnProcess } from '@automaker/platform'; + +const logger = createLogger('CherryPickService'); + +// ============================================================================ +// Types +// ============================================================================ + +export interface CherryPickOptions { + noCommit?: boolean; +} + +export interface CherryPickResult { + success: boolean; + error?: string; + hasConflicts?: boolean; + aborted?: boolean; + cherryPicked?: boolean; + commitHashes?: string[]; + branch?: string; + message?: string; +} + +// ============================================================================ +// Internal git command execution +// ============================================================================ + +/** + * Execute git command with array arguments to prevent command injection. + */ +async function execGitCommand(args: string[], cwd: string): Promise { + const result = await spawnProcess({ + command: 'git', + args, + cwd, + }); + + if (result.exitCode === 0) { + return result.stdout; + } else { + const errorMessage = result.stderr || `Git command failed with code ${result.exitCode}`; + throw new Error(errorMessage); + } +} + +// ============================================================================ +// Service Functions +// ============================================================================ + +/** + * Verify that each commit hash exists in the repository. + * + * @param worktreePath - Path to the git worktree + * @param commitHashes - Array of commit hashes to verify + * @returns The first invalid commit hash, or null if all are valid + */ +export async function verifyCommits( + worktreePath: string, + commitHashes: string[] +): Promise { + for (const hash of commitHashes) { + try { + await execGitCommand(['rev-parse', '--verify', hash], worktreePath); + } catch { + return hash; + } + } + return null; +} + +/** + * Run the cherry-pick operation on the given worktree. + * + * @param worktreePath - Path to the git worktree + * @param commitHashes - Array of commit hashes to cherry-pick (in order) + * @param options - Cherry-pick options (e.g., noCommit) + * @returns CherryPickResult with success/failure information + */ +export async function runCherryPick( + worktreePath: string, + commitHashes: string[], + options?: CherryPickOptions +): Promise { + const args = ['cherry-pick']; + if (options?.noCommit) { + args.push('--no-commit'); + } + args.push(...commitHashes); + + try { + await execGitCommand(args, worktreePath); + + const branch = await getCurrentBranch(worktreePath); + + return { + success: true, + cherryPicked: true, + commitHashes, + branch, + message: `Successfully cherry-picked ${commitHashes.length} commit(s)`, + }; + } catch (cherryPickError: unknown) { + // Check if this is a cherry-pick conflict + const err = cherryPickError as { stdout?: string; stderr?: string; message?: string }; + const output = `${err.stdout || ''} ${err.stderr || ''} ${err.message || ''}`; + const hasConflicts = + output.includes('CONFLICT') || + output.includes('cherry-pick failed') || + output.includes('could not apply'); + + if (hasConflicts) { + // Abort the cherry-pick to leave the repo in a clean state + await abortCherryPick(worktreePath); + + return { + success: false, + error: 'Cherry-pick aborted due to conflicts; no changes were applied.', + hasConflicts: true, + aborted: true, + }; + } + + // Non-conflict error - propagate + throw cherryPickError; + } +} + +/** + * Abort an in-progress cherry-pick operation. + * + * @param worktreePath - Path to the git worktree + * @returns true if abort succeeded, false if it failed (logged as warning) + */ +export async function abortCherryPick(worktreePath: string): Promise { + try { + await execGitCommand(['cherry-pick', '--abort'], worktreePath); + return true; + } catch { + logger.warn('Failed to abort cherry-pick after conflict'); + return false; + } +} + +/** + * Get the current branch name for the worktree. + * + * @param worktreePath - Path to the git worktree + * @returns The current branch name + */ +export async function getCurrentBranch(worktreePath: string): Promise { + const branchOutput = await execGitCommand(['rev-parse', '--abbrev-ref', 'HEAD'], worktreePath); + return branchOutput.trim(); +} diff --git a/apps/server/src/services/worktree-service.ts b/apps/server/src/services/worktree-service.ts new file mode 100644 index 00000000..79e5b5b1 --- /dev/null +++ b/apps/server/src/services/worktree-service.ts @@ -0,0 +1,119 @@ +/** + * WorktreeService - File-system operations for git worktrees + * + * Extracted from the worktree create route to centralise file-copy logic, + * surface errors through an EventEmitter instead of swallowing them, and + * make the behaviour testable in isolation. + */ + +import path from 'path'; +import fs from 'fs/promises'; +import type { EventEmitter } from '../lib/events.js'; +import type { SettingsService } from './settings-service.js'; + +/** + * Error thrown when one or more file copy operations fail during + * `copyConfiguredFiles`. The caller can inspect `failures` for details. + */ +export class CopyFilesError extends Error { + constructor(public readonly failures: Array<{ path: string; error: string }>) { + super(`Failed to copy ${failures.length} file(s): ${failures.map((f) => f.path).join(', ')}`); + this.name = 'CopyFilesError'; + } +} + +/** + * WorktreeService encapsulates file-system operations that run against + * git worktrees (e.g. copying project-configured files into a new worktree). + * + * All operations emit typed events so the frontend can stream progress to the + * user. Errors are collected and surfaced to the caller rather than silently + * swallowed. + */ +export class WorktreeService { + /** + * Copy files / directories listed in the project's `worktreeCopyFiles` + * setting from `projectPath` into `worktreePath`. + * + * Security: paths containing `..` segments or absolute paths are rejected. + * + * Events emitted via `emitter`: + * - `worktree:copy-files:copied` – a file or directory was successfully copied + * - `worktree:copy-files:skipped` – a source file was not found (ENOENT) + * - `worktree:copy-files:failed` – an unexpected error occurred copying a file + * + * @throws {CopyFilesError} if any copy operation fails for a reason other + * than ENOENT (missing source file). + */ + async copyConfiguredFiles( + projectPath: string, + worktreePath: string, + settingsService: SettingsService | undefined, + emitter: EventEmitter + ): Promise { + if (!settingsService) return; + + const projectSettings = await settingsService.getProjectSettings(projectPath); + const copyFiles = projectSettings.worktreeCopyFiles; + + if (!copyFiles || copyFiles.length === 0) return; + + const failures: Array<{ path: string; error: string }> = []; + + for (const relativePath of copyFiles) { + // Security: prevent path traversal + const normalized = path.normalize(relativePath); + if (normalized.startsWith('..') || path.isAbsolute(normalized)) { + const reason = 'Suspicious path rejected (traversal or absolute)'; + emitter.emit('worktree:copy-files:skipped', { + path: relativePath, + reason, + }); + continue; + } + + const sourcePath = path.join(projectPath, normalized); + const destPath = path.join(worktreePath, normalized); + + try { + // Check if source exists + const stat = await fs.stat(sourcePath); + + // Ensure destination directory exists + const destDir = path.dirname(destPath); + await fs.mkdir(destDir, { recursive: true }); + + if (stat.isDirectory()) { + // Recursively copy directory + await fs.cp(sourcePath, destPath, { recursive: true, force: true }); + } else { + // Copy single file + await fs.copyFile(sourcePath, destPath); + } + + emitter.emit('worktree:copy-files:copied', { + path: normalized, + type: stat.isDirectory() ? 'directory' : 'file', + }); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') { + emitter.emit('worktree:copy-files:skipped', { + path: normalized, + reason: 'File not found in project root', + }); + } else { + const errorMessage = err instanceof Error ? err.message : String(err); + emitter.emit('worktree:copy-files:failed', { + path: normalized, + error: errorMessage, + }); + failures.push({ path: normalized, error: errorMessage }); + } + } + } + + if (failures.length > 0) { + throw new CopyFilesError(failures); + } + } +} diff --git a/apps/server/tests/unit/services/auto-loop-coordinator.test.ts b/apps/server/tests/unit/services/auto-loop-coordinator.test.ts index 543d7825..3cb840be 100644 --- a/apps/server/tests/unit/services/auto-loop-coordinator.test.ts +++ b/apps/server/tests/unit/services/auto-loop-coordinator.test.ts @@ -886,5 +886,66 @@ describe('auto-loop-coordinator.ts', () => { expect.anything() ); }); + + it('uses pending features as fallback when loadAllFeaturesFn is null and executes eligible feature with satisfied dependencies', async () => { + // Create a completed dependency feature (will be in pendingFeatures as the allFeatures fallback) + const completedDep: Feature = { + ...testFeature, + id: 'dep-feature', + status: 'completed', + title: 'Completed Dependency', + }; + // Create a pending feature that depends on the completed dep + const pendingFeatureWithDep: Feature = { + ...testFeature, + id: 'feature-with-dep', + dependencies: ['dep-feature'], + status: 'ready', + title: 'Feature With Dependency', + }; + + // loadAllFeaturesFn is NOT provided (null) so allFeatures falls back to pendingFeatures + const coordWithoutLoadAll = new AutoLoopCoordinator( + mockEventBus, + mockConcurrencyManager, + mockSettingsService, + mockExecuteFeature, + mockLoadPendingFeatures, + mockSaveExecutionState, + mockClearExecutionState, + mockResetStuckFeatures, + mockIsFeatureFinished, + mockIsFeatureRunning + // loadAllFeaturesFn omitted (undefined/null) + ); + + // pendingFeatures includes both the completed dep and the pending feature; + // since loadAllFeaturesFn is absent, allFeatures = pendingFeatures, + // so areDependenciesSatisfied can find 'dep-feature' with status 'completed' + vi.mocked(mockLoadPendingFeatures).mockResolvedValue([completedDep, pendingFeatureWithDep]); + vi.mocked(mockConcurrencyManager.getRunningCountForWorktree).mockResolvedValue(0); + // The completed dep is finished, so it is filtered from eligible candidates; + // the pending feature with the satisfied dependency should be scheduled + vi.mocked(mockIsFeatureFinished).mockImplementation((f: Feature) => f.id === 'dep-feature'); + + await coordWithoutLoadAll.startAutoLoopForProject('/test/project', null, 1); + await vi.advanceTimersByTimeAsync(3000); + await coordWithoutLoadAll.stopAutoLoopForProject('/test/project', null); + + // The feature whose dependency is satisfied via the pending-features fallback must be executed + expect(mockExecuteFeature).toHaveBeenCalledWith( + '/test/project', + 'feature-with-dep', + true, + true + ); + // The completed dependency itself must NOT be executed (filtered by isFeatureFinishedFn) + expect(mockExecuteFeature).not.toHaveBeenCalledWith( + '/test/project', + 'dep-feature', + true, + true + ); + }); }); }); diff --git a/apps/ui/src/components/dialogs/project-file-selector-dialog.tsx b/apps/ui/src/components/dialogs/project-file-selector-dialog.tsx index e72c47f1..c19cfacd 100644 --- a/apps/ui/src/components/dialogs/project-file-selector-dialog.tsx +++ b/apps/ui/src/components/dialogs/project-file-selector-dialog.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect, useCallback, useMemo } from 'react'; +import { useState, useEffect, useCallback, useMemo, useRef } from 'react'; import { FolderOpen, Folder, @@ -70,6 +70,9 @@ export function ProjectFileSelectorDialog({ const [selectedPaths, setSelectedPaths] = useState>(new Set()); const [searchQuery, setSearchQuery] = useState(''); + // Ref to track the current request generation; incremented to cancel stale requests + const requestGenRef = useRef(0); + // Track the path segments for breadcrumb navigation const breadcrumbs = useMemo(() => { if (!currentRelativePath) return []; @@ -82,6 +85,11 @@ export function ProjectFileSelectorDialog({ const browseDirectory = useCallback( async (relativePath?: string) => { + // Increment the generation counter so any previously in-flight request + // knows it has been superseded and should not update state. + const generation = ++requestGenRef.current; + const isCancelled = () => requestGenRef.current !== generation; + setLoading(true); setError(''); setWarning(''); @@ -93,6 +101,8 @@ export function ProjectFileSelectorDialog({ relativePath: relativePath || '', }); + if (isCancelled()) return; + if (result.success) { setCurrentRelativePath(result.currentRelativePath); setParentRelativePath(result.parentRelativePath); @@ -102,9 +112,12 @@ export function ProjectFileSelectorDialog({ setError(result.error || 'Failed to browse directory'); } } catch (err) { + if (isCancelled()) return; setError(err instanceof Error ? err.message : 'Failed to load directory contents'); } finally { - setLoading(false); + if (!isCancelled()) { + setLoading(false); + } } }, [projectPath] @@ -117,6 +130,8 @@ export function ProjectFileSelectorDialog({ setSearchQuery(''); browseDirectory(); } else { + // Invalidate any in-flight request so it won't clobber the cleared state + requestGenRef.current++; setCurrentRelativePath(''); setParentRelativePath(null); setEntries([]); diff --git a/apps/ui/src/components/dialogs/sandbox-risk-dialog.tsx b/apps/ui/src/components/dialogs/sandbox-risk-dialog.tsx index 88b11d55..b42b1021 100644 --- a/apps/ui/src/components/dialogs/sandbox-risk-dialog.tsx +++ b/apps/ui/src/components/dialogs/sandbox-risk-dialog.tsx @@ -42,9 +42,9 @@ export function SandboxRiskDialog({ open, onConfirm, onDeny }: SandboxRiskDialog onEscapeKeyDown={(e) => e.preventDefault()} showCloseButton={false} > - + - + Sandbox Environment Not Detected @@ -99,7 +99,7 @@ export function SandboxRiskDialog({ open, onConfirm, onDeny }: SandboxRiskDialog - +
void; onOpenFolder: () => void; onProjectContextMenu: (project: Project, event: React.MouseEvent) => void; + setShowRemoveFromAutomakerDialog: (show: boolean) => void; } export function SidebarHeader({ @@ -32,6 +33,7 @@ export function SidebarHeader({ onNewProject, onOpenFolder, onProjectContextMenu, + setShowRemoveFromAutomakerDialog, }: SidebarHeaderProps) { const navigate = useNavigate(); const { projects, setCurrentProject } = useAppStore(); @@ -228,6 +230,22 @@ export function SidebarHeader({ Open Project + {currentProject && ( + <> + + { + setDropdownOpen(false); + setShowRemoveFromAutomakerDialog(true); + }} + className="cursor-pointer text-muted-foreground focus:text-foreground" + data-testid="collapsed-remove-from-automaker-dropdown-item" + > + + Remove from Automaker + + + )} @@ -372,6 +390,18 @@ export function SidebarHeader({ Open Project + + { + setDropdownOpen(false); + setShowRemoveFromAutomakerDialog(true); + }} + className="cursor-pointer text-muted-foreground focus:text-foreground" + data-testid="remove-from-automaker-dropdown-item" + > + + Remove from Automaker + ) : ( diff --git a/apps/ui/src/components/layout/sidebar/sidebar.tsx b/apps/ui/src/components/layout/sidebar/sidebar.tsx index 93b3532a..9cb6cd86 100644 --- a/apps/ui/src/components/layout/sidebar/sidebar.tsx +++ b/apps/ui/src/components/layout/sidebar/sidebar.tsx @@ -394,6 +394,7 @@ export function Sidebar() { onNewProject={handleNewProject} onOpenFolder={handleOpenFolder} onProjectContextMenu={handleContextMenu} + setShowRemoveFromAutomakerDialog={setShowRemoveFromAutomakerDialog} /> )} diff --git a/apps/ui/src/components/views/board-view.tsx b/apps/ui/src/components/views/board-view.tsx index 94449f8f..9aae37b7 100644 --- a/apps/ui/src/components/views/board-view.tsx +++ b/apps/ui/src/components/views/board-view.tsx @@ -68,6 +68,7 @@ import type { WorktreeInfo, MergeConflictInfo, BranchSwitchConflictInfo, + StashPopConflictInfo, } from './board-view/worktree-panel/types'; import { COLUMNS, getColumnsWithPipeline } from './board-view/constants'; import { @@ -1083,6 +1084,64 @@ export function BoardView() { [handleAddFeature, handleStartImplementation, defaultSkipTests] ); + // Handler called when checkout fails AND the stash-pop restoration produces merge conflicts. + // Creates an AI-assisted board task to guide the user through resolving the conflicts. + const handleStashPopConflict = useCallback( + async (conflictInfo: StashPopConflictInfo) => { + const description = + `Resolve merge conflicts that occurred when attempting to switch to branch "${conflictInfo.branchName}". ` + + `The checkout failed and, while restoring the previously stashed local changes, git reported merge conflicts. ` + + `${conflictInfo.stashPopConflictMessage} ` + + `Please review all conflicted files, resolve the conflicts, ensure the code compiles and tests pass, ` + + `then re-attempt the branch switch.`; + + // Create the feature + const featureData = { + title: `Resolve Stash-Pop Conflicts: branch switch to ${conflictInfo.branchName}`, + category: 'Maintenance', + description, + images: [], + imagePaths: [], + skipTests: defaultSkipTests, + model: 'opus' as const, + thinkingLevel: 'none' as const, + branchName: conflictInfo.branchName, + workMode: 'custom' as const, + priority: 1, + planningMode: 'skip' as const, + requirePlanApproval: false, + }; + + // Capture existing feature IDs before adding + const featuresBeforeIds = new Set(useAppStore.getState().features.map((f) => f.id)); + try { + await handleAddFeature(featureData); + } catch (error) { + logger.error('Failed to create stash-pop conflict resolution feature:', error); + toast.error('Failed to create feature', { + description: error instanceof Error ? error.message : 'An error occurred', + }); + return; + } + + // Find the newly created feature by looking for an ID that wasn't in the original set + const latestFeatures = useAppStore.getState().features; + const newFeature = latestFeatures.find((f) => !featuresBeforeIds.has(f.id)); + + if (newFeature) { + await handleStartImplementation(newFeature); + } else { + logger.error( + 'Could not find newly created stash-pop conflict feature to start it automatically.' + ); + toast.error('Failed to auto-start feature', { + description: 'The feature was created but could not be started automatically.', + }); + } + }, + [handleAddFeature, handleStartImplementation, defaultSkipTests] + ); + // Handler for "Make" button - creates a feature and immediately starts it const handleAddAndStartFeature = useCallback( async (featureData: Parameters[0]) => { @@ -1523,6 +1582,7 @@ export function BoardView() { onResolveConflicts={handleResolveConflicts} onCreateMergeConflictResolutionFeature={handleCreateMergeConflictResolutionFeature} onBranchSwitchConflict={handleBranchSwitchConflict} + onStashPopConflict={handleStashPopConflict} onBranchDeletedDuringMerge={(branchName) => { // Reset features that were assigned to the deleted branch (same logic as onDeleted in DeleteWorktreeDialog) hookFeatures.forEach((feature) => { diff --git a/apps/ui/src/components/views/board-view/dialogs/cherry-pick-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/cherry-pick-dialog.tsx index 5129734a..c29d9ed9 100644 --- a/apps/ui/src/components/views/board-view/dialogs/cherry-pick-dialog.tsx +++ b/apps/ui/src/components/views/board-view/dialogs/cherry-pick-dialog.tsx @@ -174,6 +174,7 @@ export function CherryPickDialog({ setCommitsError(null); setCommitLimit(30); setHasMoreCommits(false); + setLoadingBranches(false); } }, [open]); @@ -321,9 +322,7 @@ export function CherryPickDialog({ } else { // Check for conflicts const errorMessage = result.error || ''; - const hasConflicts = - errorMessage.toLowerCase().includes('conflict') || - (result as { hasConflicts?: boolean }).hasConflicts; + const hasConflicts = errorMessage.toLowerCase().includes('conflict') || result.hasConflicts; if (hasConflicts && onCreateConflictResolutionFeature) { setConflictInfo({ @@ -333,7 +332,7 @@ export function CherryPickDialog({ }); setStep('conflict'); toast.error('Cherry-pick conflicts detected', { - description: 'The cherry-pick has conflicts that need to be resolved.', + description: 'The cherry-pick was aborted due to conflicts. No changes were applied.', }); } else { toast.error('Cherry-pick failed', { @@ -359,7 +358,7 @@ export function CherryPickDialog({ }); setStep('conflict'); toast.error('Cherry-pick conflicts detected', { - description: 'The cherry-pick has conflicts that need to be resolved.', + description: 'The cherry-pick was aborted due to conflicts. No changes were applied.', }); } else { toast.error('Cherry-pick failed', { @@ -469,7 +468,7 @@ export function CherryPickDialog({ - + Cherry Pick Commits @@ -673,7 +672,7 @@ export function CherryPickDialog({ - + Cherry Pick diff --git a/apps/ui/src/components/views/board-view/dialogs/commit-worktree-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/commit-worktree-dialog.tsx index 6aa60a24..0c99a9e6 100644 --- a/apps/ui/src/components/views/board-view/dialogs/commit-worktree-dialog.tsx +++ b/apps/ui/src/components/views/board-view/dialogs/commit-worktree-dialog.tsx @@ -407,7 +407,7 @@ export function CommitWorktreeDialog({ const handleKeyDown = (e: React.KeyboardEvent) => { if ( e.key === 'Enter' && - e.metaKey && + (e.metaKey || e.ctrlKey) && !isLoading && !isGenerating && message.trim() && @@ -658,7 +658,8 @@ export function CommitWorktreeDialog({

- Press Cmd+Enter to commit + Press Cmd/Ctrl+Enter to + commit

diff --git a/apps/ui/src/components/views/board-view/dialogs/create-branch-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/create-branch-dialog.tsx index 51dc1345..5b6a5c4e 100644 --- a/apps/ui/src/components/views/board-view/dialogs/create-branch-dialog.tsx +++ b/apps/ui/src/components/views/board-view/dialogs/create-branch-dialog.tsx @@ -78,9 +78,13 @@ export function CreateBranchDialog({ if (result.success && result.result) { setBranches(result.result.branches); - // Default to current branch - if (result.result.currentBranch) { - setBaseBranch(result.result.currentBranch); + // Only set the default base branch if no branch is currently selected, + // or if the currently selected branch is no longer present in the fetched list + const branchNames = result.result.branches.map((b: BranchInfo) => b.name); + if (!baseBranch || !branchNames.includes(baseBranch)) { + if (result.result.currentBranch) { + setBaseBranch(result.result.currentBranch); + } } } } catch (err) { @@ -88,7 +92,7 @@ export function CreateBranchDialog({ } finally { setIsLoadingBranches(false); } - }, [worktree]); + }, [worktree, baseBranch]); // Reset state and fetch branches when dialog opens useEffect(() => { diff --git a/apps/ui/src/components/views/board-view/dialogs/create-pr-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/create-pr-dialog.tsx index 96a71620..7c9ed40f 100644 --- a/apps/ui/src/components/views/board-view/dialogs/create-pr-dialog.tsx +++ b/apps/ui/src/components/views/board-view/dialogs/create-pr-dialog.tsx @@ -75,6 +75,12 @@ export function CreatePRDialog({ const [remotes, setRemotes] = useState([]); const [selectedRemote, setSelectedRemote] = useState(''); const [isLoadingRemotes, setIsLoadingRemotes] = useState(false); + // Keep a ref in sync with selectedRemote so fetchRemotes can read the latest value + // without needing it in its dependency array (which would cause re-fetch loops) + const selectedRemoteRef = useRef(selectedRemote); + useEffect(() => { + selectedRemoteRef.current = selectedRemote; + }, [selectedRemote]); // Generate description state const [isGeneratingDescription, setIsGeneratingDescription] = useState(false); @@ -110,10 +116,16 @@ export function CreatePRDialog({ ); setRemotes(remoteInfos); - // Auto-select 'origin' if available, otherwise first remote + // Preserve existing selection if it's still valid; otherwise fall back to 'origin' or first remote if (remoteInfos.length > 0) { - const defaultRemote = remoteInfos.find((r) => r.name === 'origin') || remoteInfos[0]; - setSelectedRemote(defaultRemote.name); + const remoteNames = remoteInfos.map((r) => r.name); + const currentSelection = selectedRemoteRef.current; + const currentSelectionStillExists = + currentSelection !== '' && remoteNames.includes(currentSelection); + if (!currentSelectionStillExists) { + const defaultRemote = remoteInfos.find((r) => r.name === 'origin') || remoteInfos[0]; + setSelectedRemote(defaultRemote.name); + } } } } catch { diff --git a/apps/ui/src/components/views/board-view/dialogs/discard-worktree-changes-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/discard-worktree-changes-dialog.tsx index bfd8ff5e..59b7e886 100644 --- a/apps/ui/src/components/views/board-view/dialogs/discard-worktree-changes-dialog.tsx +++ b/apps/ui/src/components/views/board-view/dialogs/discard-worktree-changes-dialog.tsx @@ -319,6 +319,7 @@ export function DiscardWorktreeChangesDialog({ } } catch (err) { console.warn('Failed to load diffs for discard dialog:', err); + setError(err instanceof Error ? err.message : String(err)); } finally { setIsLoadingDiffs(false); } @@ -370,7 +371,7 @@ export function DiscardWorktreeChangesDialog({ if (result.success && result.result) { if (result.result.discarded) { - const fileCount = filesToDiscard ? filesToDiscard.length : result.result.filesDiscarded; + const fileCount = filesToDiscard ? filesToDiscard.length : selectedFiles.size; toast.success('Changes discarded', { description: `Discarded ${fileCount} ${fileCount === 1 ? 'file' : 'files'} in ${worktree.branch}`, }); diff --git a/apps/ui/src/components/views/board-view/dialogs/merge-rebase-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/merge-rebase-dialog.tsx index 921f1a9b..f2ab8bac 100644 --- a/apps/ui/src/components/views/board-view/dialogs/merge-rebase-dialog.tsx +++ b/apps/ui/src/components/views/board-view/dialogs/merge-rebase-dialog.tsx @@ -167,10 +167,15 @@ export function MergeRebaseDialog({ } }; - const handleConfirm = () => { + const handleConfirm = async () => { if (!worktree || !selectedBranch) return; - onConfirm(worktree, selectedBranch, selectedStrategy); - onOpenChange(false); + try { + await onConfirm(worktree, selectedBranch, selectedStrategy); + onOpenChange(false); + } catch (err) { + logger.error('Failed to confirm merge/rebase:', err); + throw err; + } }; const selectedRemoteData = remotes.find((r) => r.name === selectedRemote); @@ -347,7 +352,11 @@ export function MergeRebaseDialog({ className="bg-purple-600 hover:bg-purple-700 text-white" > - Merge & Rebase + {selectedStrategy === 'merge' + ? 'Merge' + : selectedStrategy === 'rebase' + ? 'Rebase' + : 'Merge & Rebase'} diff --git a/apps/ui/src/components/views/board-view/dialogs/select-remote-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/select-remote-dialog.tsx index ef1d976e..1f8e934c 100644 --- a/apps/ui/src/components/views/board-view/dialogs/select-remote-dialog.tsx +++ b/apps/ui/src/components/views/board-view/dialogs/select-remote-dialog.tsx @@ -69,6 +69,12 @@ export function SelectRemoteDialog({ url: r.url, })); setRemotes(remoteInfos); + setSelectedRemote((prev) => { + if (prev && remoteInfos.some((r) => r.name === prev)) { + return prev; + } + return remoteInfos.find((r) => r.name === 'origin')?.name ?? remoteInfos[0]?.name ?? ''; + }); } else { setError(result.error || 'Failed to fetch remotes'); } @@ -120,6 +126,12 @@ export function SelectRemoteDialog({ url: r.url, })); setRemotes(remoteInfos); + setSelectedRemote((prev) => { + if (prev && remoteInfos.some((r) => r.name === prev)) { + return prev; + } + return remoteInfos.find((r) => r.name === 'origin')?.name ?? remoteInfos[0]?.name ?? ''; + }); } else { setError(result.error || 'Failed to refresh remotes'); } diff --git a/apps/ui/src/components/views/board-view/dialogs/stash-changes-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/stash-changes-dialog.tsx index 14b53578..d6ad81c3 100644 --- a/apps/ui/src/components/views/board-view/dialogs/stash-changes-dialog.tsx +++ b/apps/ui/src/components/views/board-view/dialogs/stash-changes-dialog.tsx @@ -132,6 +132,9 @@ function parseDiff(diffText: string): ParsedFileDiff[] { for (let i = 0; i < lines.length; i++) { const line = lines[i]; + // Skip trailing empty string produced by a final newline in diffText + if (line === '' && i === lines.length - 1) continue; + if (line.startsWith('diff --git')) { if (currentFile) { if (currentHunk) currentFile.hunks.push(currentHunk); diff --git a/apps/ui/src/components/views/board-view/dialogs/view-commits-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/view-commits-dialog.tsx index 7125130e..30ba9af2 100644 --- a/apps/ui/src/components/views/board-view/dialogs/view-commits-dialog.tsx +++ b/apps/ui/src/components/views/board-view/dialogs/view-commits-dialog.tsx @@ -47,7 +47,9 @@ interface ViewCommitsDialogProps { } function formatRelativeDate(dateStr: string): string { + if (!dateStr) return 'unknown date'; const date = new Date(dateStr); + if (isNaN(date.getTime())) return 'unknown date'; const now = new Date(); const diffMs = now.getTime() - date.getTime(); const diffSecs = Math.floor(diffMs / 1000); diff --git a/apps/ui/src/components/views/board-view/hooks/use-board-actions.ts b/apps/ui/src/components/views/board-view/hooks/use-board-actions.ts index 326d64e4..5a716f50 100644 --- a/apps/ui/src/components/views/board-view/hooks/use-board-actions.ts +++ b/apps/ui/src/components/views/board-view/hooks/use-board-actions.ts @@ -280,6 +280,8 @@ export function useBoardActions({ }); } } + + return createdFeature; }, [ addFeature, @@ -1221,13 +1223,12 @@ export function useBoardActions({ dependencies: [parentFeature.id], }; - await handleAddFeature(duplicatedFeatureData); + const newFeature = await handleAddFeature(duplicatedFeatureData); - // Get the newly created feature (last added feature) to use as parent for next iteration - const currentFeatures = useAppStore.getState().features; - const newestFeature = currentFeatures[currentFeatures.length - 1]; - if (newestFeature) { - parentFeature = newestFeature; + // Use the returned feature directly as the parent for the next iteration, + // avoiding a fragile assumption that the newest feature is the last item in the store + if (newFeature) { + parentFeature = newFeature; } } 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 2600d57d..0ce327cc 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 @@ -451,31 +451,28 @@ export function WorktreeActionsDropdown({ )} {/* Stash operations - combined submenu */} {(onStashChanges || onViewStashes) && ( - +
{/* Main clickable area - stash changes (primary action) */} { - if (!gitRepoStatus.isGitRepo) return; + if (!canPerformGitOps) return; if (worktree.hasChanges && onStashChanges) { onStashChanges(worktree); } else if (onViewStashes) { onViewStashes(worktree); } }} - disabled={!gitRepoStatus.isGitRepo} + disabled={!canPerformGitOps} className={cn( 'text-xs flex-1 pr-0 rounded-r-none', - !gitRepoStatus.isGitRepo && 'opacity-50 cursor-not-allowed' + !canPerformGitOps && 'opacity-50 cursor-not-allowed' )} > {worktree.hasChanges && onStashChanges ? 'Stash Changes' : 'Stashes'} - {!gitRepoStatus.isGitRepo && ( + {!canPerformGitOps && ( )} @@ -483,9 +480,9 @@ export function WorktreeActionsDropdown({
diff --git a/apps/ui/src/components/views/board-view/worktree-panel/hooks/use-worktree-actions.ts b/apps/ui/src/components/views/board-view/worktree-panel/hooks/use-worktree-actions.ts index 34f54a85..9d83281a 100644 --- a/apps/ui/src/components/views/board-view/worktree-panel/hooks/use-worktree-actions.ts +++ b/apps/ui/src/components/views/board-view/worktree-panel/hooks/use-worktree-actions.ts @@ -20,6 +20,12 @@ interface UseWorktreeActionsOptions { branchName: string; previousBranch: string; }) => void; + /** Callback when checkout fails AND the stash-pop restoration produces merge conflicts */ + onStashPopConflict?: (info: { + worktreePath: string; + branchName: string; + stashPopConflictMessage: string; + }) => void; } export function useWorktreeActions(options?: UseWorktreeActionsOptions) { @@ -29,6 +35,7 @@ export function useWorktreeActions(options?: UseWorktreeActionsOptions) { // Use React Query mutations const switchBranchMutation = useSwitchBranch({ onConflict: options?.onBranchSwitchConflict, + onStashPopConflict: options?.onStashPopConflict, }); const pullMutation = usePullWorktree(); const pushMutation = usePushWorktree(); 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 21f539fb..05c88fe2 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 @@ -86,6 +86,13 @@ export interface BranchSwitchConflictInfo { previousBranch: string; } +/** Info passed when a checkout failure triggers a stash-pop that itself produces conflicts */ +export interface StashPopConflictInfo { + worktreePath: string; + branchName: string; + stashPopConflictMessage: string; +} + export interface WorktreePanelProps { projectPath: string; onCreateWorktree: () => void; @@ -98,6 +105,8 @@ export interface WorktreePanelProps { onCreateMergeConflictResolutionFeature?: (conflictInfo: MergeConflictInfo) => void; /** Called when branch switch stash reapply results in merge conflicts */ onBranchSwitchConflict?: (conflictInfo: BranchSwitchConflictInfo) => void; + /** Called when checkout fails and the stash-pop restoration itself produces merge conflicts */ + onStashPopConflict?: (conflictInfo: StashPopConflictInfo) => void; /** Called when a branch is deleted during merge - features should be reassigned to main */ onBranchDeletedDuringMerge?: (branchName: string) => void; onRemovedWorktrees?: (removedWorktrees: Array<{ path: string; branch: string }>) => void; 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 c941b35d..3704409a 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 @@ -60,6 +60,7 @@ export function WorktreePanel({ onResolveConflicts, onCreateMergeConflictResolutionFeature, onBranchSwitchConflict, + onStashPopConflict, onBranchDeletedDuringMerge, onRemovedWorktrees, runningFeatureIds = [], @@ -113,6 +114,7 @@ export function WorktreePanel({ handleOpenInExternalTerminal, } = useWorktreeActions({ onBranchSwitchConflict: onBranchSwitchConflict, + onStashPopConflict: onStashPopConflict, }); const { hasRunningFeatures } = useRunningFeatures({ @@ -563,8 +565,12 @@ export function WorktreePanel({ setSelectRemoteWorktree(worktree); setSelectRemoteOperation('pull'); setSelectRemoteDialogOpen(true); + } else if (result.success && result.result && result.result.remotes.length === 1) { + // Exactly one remote - use it directly + const remoteName = result.result.remotes[0].name; + handlePull(worktree, remoteName); } else { - // Single or no remote - proceed with default behavior + // No remotes - proceed with default behavior handlePull(worktree); } } catch { @@ -587,8 +593,12 @@ export function WorktreePanel({ setSelectRemoteWorktree(worktree); setSelectRemoteOperation('push'); setSelectRemoteDialogOpen(true); + } else if (result.success && result.result && result.result.remotes.length === 1) { + // Exactly one remote - use it directly + const remoteName = result.result.remotes[0].name; + handlePush(worktree, remoteName); } else { - // Single or no remote - proceed with default behavior + // No remotes - proceed with default behavior handlePush(worktree); } } catch { diff --git a/apps/ui/src/components/views/project-settings-view/worktree-preferences-section.tsx b/apps/ui/src/components/views/project-settings-view/worktree-preferences-section.tsx index 813b2ff5..3792185b 100644 --- a/apps/ui/src/components/views/project-settings-view/worktree-preferences-section.tsx +++ b/apps/ui/src/components/views/project-settings-view/worktree-preferences-section.tsx @@ -47,7 +47,7 @@ export function WorktreePreferencesSection({ project }: WorktreePreferencesSecti const setDefaultDeleteBranch = useAppStore((s) => s.setDefaultDeleteBranch); const getAutoDismissInitScriptIndicator = useAppStore((s) => s.getAutoDismissInitScriptIndicator); const setAutoDismissInitScriptIndicator = useAppStore((s) => s.setAutoDismissInitScriptIndicator); - const getWorktreeCopyFiles = useAppStore((s) => s.getWorktreeCopyFiles); + const copyFiles = useAppStore((s) => s.worktreeCopyFilesByProject[project.path] ?? []); const setWorktreeCopyFiles = useAppStore((s) => s.setWorktreeCopyFiles); // Get effective worktrees setting (project override or global fallback) @@ -64,7 +64,6 @@ export function WorktreePreferencesSection({ project }: WorktreePreferencesSecti // Copy files state const [newCopyFilePath, setNewCopyFilePath] = useState(''); const [fileSelectorOpen, setFileSelectorOpen] = useState(false); - const copyFiles = getWorktreeCopyFiles(project.path); // Get the current settings for this project const showIndicator = getShowInitScriptIndicator(project.path); @@ -245,15 +244,15 @@ export function WorktreePreferencesSection({ project }: WorktreePreferencesSecti if (!normalized) return; // Check for duplicates - const currentFiles = getWorktreeCopyFiles(project.path); - if (currentFiles.includes(normalized)) { + if (copyFiles.includes(normalized)) { toast.error('File already in list', { description: `"${normalized}" is already configured for copying.`, }); return; } - const updatedFiles = [...currentFiles, normalized]; + const prevFiles = copyFiles; + const updatedFiles = [...copyFiles, normalized]; setWorktreeCopyFiles(project.path, updatedFiles); setNewCopyFilePath(''); @@ -267,16 +266,19 @@ export function WorktreePreferencesSection({ project }: WorktreePreferencesSecti description: `"${normalized}" will be copied to new worktrees.`, }); } catch (error) { + // Rollback optimistic update on failure + setWorktreeCopyFiles(project.path, prevFiles); + setNewCopyFilePath(normalized); console.error('Failed to persist worktreeCopyFiles:', error); toast.error('Failed to save copy files setting'); } - }, [project.path, newCopyFilePath, getWorktreeCopyFiles, setWorktreeCopyFiles]); + }, [project.path, newCopyFilePath, copyFiles, setWorktreeCopyFiles]); // Remove a file path from copy list const handleRemoveCopyFile = useCallback( async (filePath: string) => { - const currentFiles = getWorktreeCopyFiles(project.path); - const updatedFiles = currentFiles.filter((f) => f !== filePath); + const prevFiles = copyFiles; + const updatedFiles = copyFiles.filter((f) => f !== filePath); setWorktreeCopyFiles(project.path, updatedFiles); // Persist to server @@ -287,26 +289,27 @@ export function WorktreePreferencesSection({ project }: WorktreePreferencesSecti }); toast.success('Copy file removed'); } catch (error) { + // Rollback optimistic update on failure + setWorktreeCopyFiles(project.path, prevFiles); console.error('Failed to persist worktreeCopyFiles:', error); toast.error('Failed to save copy files setting'); } }, - [project.path, getWorktreeCopyFiles, setWorktreeCopyFiles] + [project.path, copyFiles, setWorktreeCopyFiles] ); // Handle files selected from the file selector dialog const handleFileSelectorSelect = useCallback( async (paths: string[]) => { - const currentFiles = getWorktreeCopyFiles(project.path); - // Filter out duplicates - const newPaths = paths.filter((p) => !currentFiles.includes(p)); + const newPaths = paths.filter((p) => !copyFiles.includes(p)); if (newPaths.length === 0) { toast.info('All selected files are already in the list'); return; } - const updatedFiles = [...currentFiles, ...newPaths]; + const prevFiles = copyFiles; + const updatedFiles = [...copyFiles, ...newPaths]; setWorktreeCopyFiles(project.path, updatedFiles); // Persist to server @@ -319,11 +322,13 @@ export function WorktreePreferencesSection({ project }: WorktreePreferencesSecti description: newPaths.map((p) => `"${p}"`).join(', '), }); } catch (error) { + // Rollback optimistic update on failure + setWorktreeCopyFiles(project.path, prevFiles); console.error('Failed to persist worktreeCopyFiles:', error); toast.error('Failed to save copy files setting'); } }, - [project.path, getWorktreeCopyFiles, setWorktreeCopyFiles] + [project.path, copyFiles, setWorktreeCopyFiles] ); return ( diff --git a/apps/ui/src/hooks/mutations/use-worktree-mutations.ts b/apps/ui/src/hooks/mutations/use-worktree-mutations.ts index f9e6efc3..f406aeab 100644 --- a/apps/ui/src/hooks/mutations/use-worktree-mutations.ts +++ b/apps/ui/src/hooks/mutations/use-worktree-mutations.ts @@ -298,11 +298,21 @@ export function useMergeWorktree(projectPath: string) { * If the reapply causes merge conflicts, the onConflict callback is called so * the UI can create a conflict resolution task. * - * @param options.onConflict - Callback when merge conflicts occur after stash reapply + * If the checkout itself fails and the stash-pop used to restore changes also + * produces conflicts, the onStashPopConflict callback is called so the UI can + * create an AI-assisted conflict resolution task on the board. + * + * @param options.onConflict - Callback when merge conflicts occur after stash reapply (success path) + * @param options.onStashPopConflict - Callback when checkout fails AND stash-pop restoration has conflicts * @returns Mutation for switching branches */ export function useSwitchBranch(options?: { onConflict?: (info: { worktreePath: string; branchName: string; previousBranch: string }) => void; + onStashPopConflict?: (info: { + worktreePath: string; + branchName: string; + stashPopConflictMessage: string; + }) => void; }) { const queryClient = useQueryClient(); @@ -318,6 +328,47 @@ export function useSwitchBranch(options?: { if (!api.worktree) throw new Error('Worktree API not available'); const result = await api.worktree.switchBranch(worktreePath, branchName); if (!result.success) { + // When the checkout failed and restoring the stash produced conflicts, surface + // this as a structured error so the caller can create a board task for resolution. + if (result.stashPopConflicts) { + const conflictError = new Error(result.error || 'Failed to switch branch'); + // Attach the extra metadata so onError can forward it to the callback. + ( + conflictError as Error & { + stashPopConflicts: boolean; + stashPopConflictMessage: string; + worktreePath: string; + branchName: string; + } + ).stashPopConflicts = true; + ( + conflictError as Error & { + stashPopConflicts: boolean; + stashPopConflictMessage: string; + worktreePath: string; + branchName: string; + } + ).stashPopConflictMessage = + result.stashPopConflictMessage ?? + 'Stash pop resulted in conflicts: please resolve conflicts before retrying.'; + ( + conflictError as Error & { + stashPopConflicts: boolean; + stashPopConflictMessage: string; + worktreePath: string; + branchName: string; + } + ).worktreePath = worktreePath; + ( + conflictError as Error & { + stashPopConflicts: boolean; + stashPopConflictMessage: string; + worktreePath: string; + branchName: string; + } + ).branchName = branchName; + throw conflictError; + } throw new Error(result.error || 'Failed to switch branch'); } if (!result.result) { @@ -345,9 +396,38 @@ export function useSwitchBranch(options?: { } }, onError: (error: Error) => { - toast.error('Failed to switch branch', { - description: error.message, - }); + const enrichedError = error as Error & { + stashPopConflicts?: boolean; + stashPopConflictMessage?: string; + worktreePath?: string; + branchName?: string; + }; + + if ( + enrichedError.stashPopConflicts && + enrichedError.worktreePath && + enrichedError.branchName + ) { + // Checkout failed AND the stash-pop produced conflicts — notify the UI so it + // can create an AI-assisted board task to guide the user through resolution. + toast.error('Branch switch failed with stash conflicts', { + description: + enrichedError.stashPopConflictMessage ?? + 'Stash pop resulted in conflicts. Please resolve the conflicts in your working tree.', + duration: 10000, + }); + options?.onStashPopConflict?.({ + worktreePath: enrichedError.worktreePath, + branchName: enrichedError.branchName, + stashPopConflictMessage: + enrichedError.stashPopConflictMessage ?? + 'Stash pop resulted in conflicts. Please resolve the conflicts in your working tree.', + }); + } else { + toast.error('Failed to switch branch', { + description: error.message, + }); + } }, }); } diff --git a/apps/ui/src/hooks/use-provider-auth-init.ts b/apps/ui/src/hooks/use-provider-auth-init.ts index c95ce9ea..ec0a75b0 100644 --- a/apps/ui/src/hooks/use-provider-auth-init.ts +++ b/apps/ui/src/hooks/use-provider-auth-init.ts @@ -148,43 +148,48 @@ export function useProviderAuthInit() { // 4. Gemini Auth Status try { const result = await api.setup.getGeminiStatus(); - if (result.success) { - // Set CLI status + + // Always set CLI status if any CLI info is available + if ( + result.installed !== undefined || + result.version !== undefined || + result.path !== undefined + ) { setGeminiCliStatus({ installed: result.installed ?? false, version: result.version, path: result.path, }); + } - // Set Auth status - always set a status to mark initialization as complete - if (result.auth) { - const auth = result.auth; - const validMethods: GeminiAuthStatus['method'][] = [ - 'google_login', - 'api_key', - 'vertex_ai', - 'none', - ]; + // Always set auth status regardless of result.success + if (result.success && result.auth) { + const auth = result.auth; + const validMethods: GeminiAuthStatus['method'][] = [ + 'google_login', + 'api_key', + 'vertex_ai', + 'none', + ]; - const method = validMethods.includes(auth.method as GeminiAuthStatus['method']) - ? (auth.method as GeminiAuthStatus['method']) - : ((auth.authenticated ? 'google_login' : 'none') as GeminiAuthStatus['method']); + const method = validMethods.includes(auth.method as GeminiAuthStatus['method']) + ? (auth.method as GeminiAuthStatus['method']) + : ((auth.authenticated ? 'google_login' : 'none') as GeminiAuthStatus['method']); - setGeminiAuthStatus({ - authenticated: auth.authenticated, - method, - hasApiKey: auth.hasApiKey ?? false, - hasEnvApiKey: auth.hasEnvApiKey ?? false, - }); - } else { - // No auth info available, set default unauthenticated status - setGeminiAuthStatus({ - authenticated: false, - method: 'none', - hasApiKey: false, - hasEnvApiKey: false, - }); - } + setGeminiAuthStatus({ + authenticated: auth.authenticated, + method, + hasApiKey: auth.hasApiKey ?? false, + hasEnvApiKey: auth.hasEnvApiKey ?? false, + }); + } else { + // result.success is false or result.auth is missing — set default unauthenticated status + setGeminiAuthStatus({ + authenticated: false, + method: 'none', + hasApiKey: false, + hasEnvApiKey: false, + }); } } catch (error) { logger.error('Failed to init Gemini auth status:', error); diff --git a/apps/ui/src/types/electron.d.ts b/apps/ui/src/types/electron.d.ts index 3c2bb6fe..bb8b31a4 100644 --- a/apps/ui/src/types/electron.d.ts +++ b/apps/ui/src/types/electron.d.ts @@ -1034,6 +1034,10 @@ export interface WorktreeAPI { }; error?: string; code?: 'NOT_GIT_REPO' | 'NO_COMMITS' | 'UNCOMMITTED_CHANGES'; + /** True when the checkout failed AND the stash-pop used to restore changes produced merge conflicts */ + stashPopConflicts?: boolean; + /** Human-readable message describing the stash-pop conflict situation */ + stashPopConflictMessage?: string; }>; // List all remotes and their branches @@ -1524,6 +1528,7 @@ export interface WorktreeAPI { }; error?: string; hasConflicts?: boolean; + aborted?: boolean; }>; // Get commit log for a specific branch (not just the current one) diff --git a/libs/types/src/event.ts b/libs/types/src/event.ts index d11bfd07..c4c84693 100644 --- a/libs/types/src/event.ts +++ b/libs/types/src/event.ts @@ -40,6 +40,9 @@ export type EventType = | 'ideation:idea-updated' | 'ideation:idea-deleted' | 'ideation:idea-converted' + | 'worktree:copy-files:copied' + | 'worktree:copy-files:skipped' + | 'worktree:copy-files:failed' | 'worktree:init-started' | 'worktree:init-output' | 'worktree:init-completed' @@ -53,6 +56,14 @@ export type EventType = | 'test-runner:completed' | 'test-runner:error' | 'test-runner:result' + | 'cherry-pick:started' + | 'cherry-pick:success' + | 'cherry-pick:conflict' + | 'cherry-pick:failure' + | 'branchCommitLog:start' + | 'branchCommitLog:progress' + | 'branchCommitLog:done' + | 'branchCommitLog:error' | 'notification:created'; export type EventCallback = (type: EventType, payload: unknown) => void;