diff --git a/apps/server/src/services/branch-utils.ts b/apps/server/src/services/branch-utils.ts index a2e1c3c0..9a618da0 100644 --- a/apps/server/src/services/branch-utils.ts +++ b/apps/server/src/services/branch-utils.ts @@ -23,6 +23,13 @@ export interface HasAnyChangesOptions { * considered as real working-tree changes (e.g. worktree-branch-service). */ excludeWorktreePaths?: boolean; + /** + * When true (default), untracked files (lines starting with "??") are + * included in the change count. When false, untracked files are ignored so + * that hasAnyChanges() is consistent with stashChanges() called without + * --include-untracked. + */ + includeUntracked?: boolean; } // ============================================================================ @@ -43,15 +50,20 @@ function isExcludedWorktreeLine(line: string): boolean { // ============================================================================ /** - * Check if there are any changes (including untracked) that should be stashed. + * Check if there are any changes that should be stashed. * * @param cwd - Working directory of the git repository / worktree * @param options - Optional flags controlling which lines are counted * @param options.excludeWorktreePaths - When true, lines matching worktree * internal paths are excluded so they are not mistaken for real changes + * @param options.includeUntracked - When false, untracked files (lines + * starting with "??") are excluded so this is consistent with a + * stashChanges() call that does not pass --include-untracked. + * Defaults to true. */ export async function hasAnyChanges(cwd: string, options?: HasAnyChangesOptions): Promise { try { + const includeUntracked = options?.includeUntracked ?? true; const stdout = await execGitCommand(['status', '--porcelain'], cwd); const lines = stdout .trim() @@ -59,6 +71,7 @@ export async function hasAnyChanges(cwd: string, options?: HasAnyChangesOptions) .filter((line) => { if (!line.trim()) return false; if (options?.excludeWorktreePaths && isExcludedWorktreeLine(line)) return false; + if (!includeUntracked && line.startsWith('??')) return false; return true; }); return lines.length > 0; diff --git a/apps/server/src/services/checkout-branch-service.ts b/apps/server/src/services/checkout-branch-service.ts index f4b9c817..35fa8f21 100644 --- a/apps/server/src/services/checkout-branch-service.ts +++ b/apps/server/src/services/checkout-branch-service.ts @@ -133,7 +133,7 @@ export async function performCheckoutBranch( let didStash = false; if (shouldStash) { - const hadChanges = await hasAnyChanges(worktreePath); + const hadChanges = await hasAnyChanges(worktreePath, { includeUntracked }); if (hadChanges) { events?.emit('switch:stash', { worktreePath, @@ -187,14 +187,31 @@ export async function performCheckoutBranch( action: 'pop', }); - const popResult = await popStash(worktreePath); - hasConflicts = popResult.hasConflicts; - if (popResult.hasConflicts) { - conflictMessage = `Created branch '${branchName}' but merge conflicts occurred when reapplying your local changes. Please resolve the conflicts.`; - } else if (!popResult.success) { - conflictMessage = `Created branch '${branchName}' but failed to reapply stashed changes: ${popResult.error}. Your changes are still in the stash.`; - } else { - stashReapplied = true; + // Isolate the pop in its own try/catch so a thrown exception does not + // propagate to the outer catch block, which would attempt a second pop. + try { + const popResult = await popStash(worktreePath); + // Mark didStash false so the outer error-recovery path cannot pop again. + didStash = false; + hasConflicts = popResult.hasConflicts; + if (popResult.hasConflicts) { + conflictMessage = `Created branch '${branchName}' but merge conflicts occurred when reapplying your local changes. Please resolve the conflicts.`; + } else if (!popResult.success) { + conflictMessage = `Created branch '${branchName}' but failed to reapply stashed changes: ${popResult.error}. Your changes are still in the stash.`; + } else { + stashReapplied = true; + } + } catch (popError) { + // Pop threw an unexpected exception. Record the error and clear didStash + // so the outer catch does not attempt a second pop. + didStash = false; + conflictMessage = `Created branch '${branchName}' but an error occurred while reapplying stashed changes: ${getErrorMessage(popError)}. Your changes may still be in the stash.`; + events?.emit('switch:pop', { + worktreePath, + targetBranch: branchName, + action: 'pop', + error: getErrorMessage(popError), + }); } } @@ -255,28 +272,49 @@ export async function performCheckoutBranch( } catch (checkoutError) { // 7. If checkout failed and we stashed, try to restore the stash if (didStash) { - const popResult = await popStash(worktreePath); - if (popResult.hasConflicts) { - const checkoutErrorMsg = getErrorMessage(checkoutError); - events?.emit('switch:error', { - worktreePath, - branchName, - error: checkoutErrorMsg, - stashPopConflicts: true, - }); - return { - 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.', - }; - } else if (!popResult.success) { + try { + const popResult = await popStash(worktreePath); + if (popResult.hasConflicts) { + const checkoutErrorMsg = getErrorMessage(checkoutError); + events?.emit('switch:error', { + worktreePath, + branchName, + error: checkoutErrorMsg, + stashPopConflicts: true, + }); + return { + 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.', + }; + } else if (!popResult.success) { + 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.`; + events?.emit('switch:error', { + worktreePath, + branchName, + error: combinedMessage, + }); + return { + success: false, + error: combinedMessage, + stashPopConflicts: false, + }; + } + // popResult.success === true: stash was cleanly restored + } catch (popError) { + // popStash itself threw — build a failure result rather than letting + // the exception propagate and produce an unhandled rejection. const checkoutErrorMsg = getErrorMessage(checkoutError); + const popErrorMsg = getErrorMessage(popError); const combinedMessage = - `${checkoutErrorMsg}. Additionally, restoring your stashed changes failed: ` + - `${popResult.error ?? 'unknown error'} — your changes are still saved in the stash.`; + `${checkoutErrorMsg}. Additionally, an error occurred while attempting to restore ` + + `your stashed changes: ${popErrorMsg} — your changes may still be saved in the stash.`; events?.emit('switch:error', { worktreePath, branchName, @@ -286,9 +324,9 @@ export async function performCheckoutBranch( success: false, error: combinedMessage, stashPopConflicts: false, + stashPopConflictMessage: combinedMessage, }; } - // popResult.success === true: stash was cleanly restored } const checkoutErrorMsg = getErrorMessage(checkoutError); events?.emit('switch:error', { diff --git a/apps/server/src/services/rebase-service.ts b/apps/server/src/services/rebase-service.ts index 05c2f33e..14f806e9 100644 --- a/apps/server/src/services/rebase-service.ts +++ b/apps/server/src/services/rebase-service.ts @@ -149,10 +149,23 @@ export async function runRebase(worktreePath: string, ontoBranch: string): Promi const hasConflicts = textIndicatesConflict || rebaseStateExists || hasUnmergedPaths; if (hasConflicts) { - // Get list of conflicted files - const conflictFiles = await getConflictFiles(worktreePath); + // Attempt to fetch the list of conflicted files. We wrap this in its + // own try/catch so that a failure here does NOT prevent abortRebase from + // running – keeping the repository in a clean state is the priority. + let conflictFiles: string[] | undefined; + let conflictFilesError: unknown; + try { + conflictFiles = await getConflictFiles(worktreePath); + } catch (getConflictFilesError: unknown) { + conflictFilesError = getConflictFilesError; + logger.warn('Failed to retrieve conflict files after rebase conflict', { + worktreePath, + error: getErrorMessage(getConflictFilesError), + }); + } - // Abort the rebase to leave the repo in a clean state + // Abort the rebase to leave the repo in a clean state. This must + // always run regardless of whether getConflictFiles succeeded. const aborted = await abortRebase(worktreePath); if (!aborted) { @@ -161,6 +174,20 @@ export async function runRebase(worktreePath: string, ontoBranch: string): Promi }); } + // Re-throw a composed error so callers retain both the original rebase + // failure context and any conflict-file lookup failure. + if (conflictFilesError !== undefined) { + const composedMessage = [ + `Rebase of "${currentBranch}" onto "${normalizedOntoBranch}" failed due to conflicts.`, + `Original rebase error: ${getErrorMessage(rebaseError)}`, + `Additionally, fetching conflict files failed: ${getErrorMessage(conflictFilesError)}`, + aborted + ? 'The rebase was aborted; no changes were applied.' + : 'The rebase abort also failed; repository may be in a dirty state.', + ].join(' '); + throw new Error(composedMessage); + } + return { success: false, error: aborted