fix sync and resursive rebase

src/commands/sync.tsx

@@ -1,5 +1,5 @@
 import ErrorDisplay from '../components/error-display.js';
-import React, { useCallback, useState } from 'react';
+import React, { useCallback, useEffect, useMemo, useState } from 'react';
 import { Action, useAction } from '../hooks/use-action.js';
 import { CommandConfig, CommandProps } from '../types.js';
 import { ConfirmStatement } from '../components/confirm-statement.js';
@@ -93,11 +93,18 @@ const useSyncAction = ({
     rootBranchName: string;
 }): UseSyncActionResult => {
     const git = useGit();
-    const { currentTree, removeBranch } = useTree();
+    const { removeBranch, get } = useTree();
     const [allContestedBranches, setAllContestedBranches] = useState<string[]>(
         []
     );
 
+    /**
+     * We need a snapshot of the tree instead of "currentTree", because deleting branches while doing cleanup will
+     * cause the "currentTree" to change, which problematically tries to re-trigger the whole sync process in the
+     * middle, which causes errors.
+     */
+    const currentTreeSnapshot = useMemo(() => get(), []);
+
     const skipContestedBranch = useCallback((branch: string) => {
         setAllContestedBranches((prev) => prev.filter((b) => b !== branch));
     }, []);
@@ -114,18 +121,26 @@ const useSyncAction = ({
 
     const performAction = useCallback(async () => {
         // todo: unsure if this is the correct condition
-        if (!currentTree.length) return;
+        if (!currentTreeSnapshot.length) return;
 
         await git.checkout(rootBranchName);
         await git.pull();
+        const contestedBranches = [];
 
-        for (const node of currentTree) {
+        for (const node of currentTreeSnapshot) {
             const closedOnRemote = await git.isClosedOnRemote(node.key);
             if (closedOnRemote) {
-                setAllContestedBranches((prev) => [...prev, node.key]);
+                contestedBranches.push(node.key);
             }
         }
-    }, [git, currentTree]);
+
+        /**
+         * We need to update the state of contested branches all at once to prevent the user attempting to run the
+         * branch deletion function while a git.isClosedOnRemote() is running. Git does not allow multiple commands
+         * to be run in parallel and enforces this with an internal lockfile.
+         */
+        setAllContestedBranches(contestedBranches);
+    }, [git, currentTreeSnapshot]);
 
     const action = useAction({
         asyncAction: performAction,

src/contexts/tree-display.context.tsx

@@ -1,18 +1,12 @@
-import React, {
-    ReactNode,
-    createContext,
-    useCallback,
-    useContext,
-    useMemo,
-} from 'react';
+import React, { ReactNode, createContext, useContext, useMemo } from 'react';
 import {
     DisplayNode,
     getDisplayNodes,
     maxWidthFromDisplayNodes,
 } from '../utils/tree-display.js';
 import { treeToParentChildRecord } from '../utils/tree-helpers.js';
-import { useAsyncValue } from '../hooks/use-async-value.js';
-import { useGit } from '../hooks/use-git.js';
+import { useBranchNeedsRebaseRecord } from '../hooks/computed-values/use-branch-needs-rebase-record.js';
+import { useCleanCurrentTree } from '../hooks/computed-values/use-clean-current-tree.js';
 import { useTree } from '../hooks/use-tree.js';
 
 interface TreeDisplayContextType {
@@ -30,43 +24,23 @@ const TreeDisplayContext = createContext<TreeDisplayContextType>({
 });
 
 export const TreeDisplayProvider = ({ children }: { children: ReactNode }) => {
-    const git = useGit();
-    const { rootBranchName, currentTree } = useTree();
+    const { rootBranchName } = useTree();
+
+    const { value: currentTree, isLoading: isLoadingCurrentTree } =
+        useCleanCurrentTree();
+
     const treeParentChildRecord = useMemo(
         () => treeToParentChildRecord(currentTree),
         [currentTree]
     );
-
-    const getBranchNeedsRebaseRecord = useCallback(async () => {
-        const record: Record<string, boolean> = {};
-        await Promise.all(
-            currentTree.map(async (_node) => {
-                if (!_node.parent) return null;
-
-                record[_node.key] = await git.needsRebaseOnto({
-                    branch: _node.key,
-                    ontoBranch: _node.parent,
-                });
-                return null;
-            })
-        );
-        return record;
-    }, [currentTree, git.needsRebaseOnto]);
-
-    const branchNeedsRebaseRecordResult = useAsyncValue({
-        getValue: getBranchNeedsRebaseRecord,
-    });
-
-    const branchNeedsRebaseRecord = useMemo(() => {
-        if (branchNeedsRebaseRecordResult.isLoading)
-            return {} as Record<string, boolean>;
-
-        return branchNeedsRebaseRecordResult.value;
-    }, [branchNeedsRebaseRecordResult]);
+    const {
+        value: branchNeedsRebaseRecord,
+        isLoading: isLoadingBranchNeedsRebaseRecord,
+    } = useBranchNeedsRebaseRecord({ currentTree });
 
     const isLoading = useMemo(() => {
-        return branchNeedsRebaseRecordResult.isLoading;
-    }, [branchNeedsRebaseRecordResult]);
+        return isLoadingBranchNeedsRebaseRecord || isLoadingCurrentTree;
+    }, [isLoadingBranchNeedsRebaseRecord, isLoadingCurrentTree]);
 
     const nodes: DisplayNode[] = rootBranchName
         ? getDisplayNodes({

src/hooks/computed-values/use-branch-needs-rebase-record.ts

@@ -0,0 +1,34 @@
+import { Tree } from '../../services/tree.js';
+import { useAsyncValueWithDefault } from '../use-async-value.js';
+import { useCallback, useMemo } from 'react';
+import { useGit } from '../use-git.js';
+
+export const useBranchNeedsRebaseRecord = ({
+    currentTree,
+}: {
+    currentTree: Tree;
+}) => {
+    const git = useGit();
+
+    const getBranchNeedsRebaseRecord = useCallback(async () => {
+        const record: Record<string, boolean> = {};
+
+        for (const _node of currentTree) {
+            if (!_node.parent) continue;
+
+            record[_node.key] = await git.needsRebaseOnto({
+                branch: _node.key,
+                ontoBranch: _node.parent,
+            });
+        }
+        return record;
+    }, [currentTree, git.needsRebaseOnto]);
+
+    // We need to memoize the default value to avoid infinite renders
+    const defaultValue = useMemo(() => ({}), []);
+
+    return useAsyncValueWithDefault({
+        getValue: getBranchNeedsRebaseRecord,
+        defaultValue,
+    });
+};

src/hooks/computed-values/use-clean-current-tree.ts

@@ -0,0 +1,34 @@
+import { Tree } from '../../services/tree.js';
+import { useAsyncValueWithDefault } from '../use-async-value.js';
+import { useCallback, useMemo } from 'react';
+import { useGit } from '../use-git.js';
+import { useTree } from '../use-tree.js';
+
+/**
+ * The "cleaned" tree is the current tree without the branches that don't exist locally.
+ */
+export const useCleanCurrentTree = () => {
+    const git = useGit();
+    const { currentTree: uncleanCurrentTree, removeBranch } = useTree();
+
+    const getCleanCurrentTree = useCallback(async () => {
+        const cleanTree: Tree = [];
+        for (const _node of uncleanCurrentTree) {
+            const branchExistsLocally = await git.branchExistsLocally(
+                _node.key
+            );
+
+            if (branchExistsLocally) cleanTree.push(_node);
+            else removeBranch(_node.key, { ignoreBranchDoesNotExist: true });
+        }
+        return cleanTree;
+    }, [uncleanCurrentTree, git.branchExistsLocally, removeBranch]);
+
+    // We need to memoize the default value to avoid infinite renders
+    const defaultValue = useMemo(() => [], []);
+
+    return useAsyncValueWithDefault({
+        getValue: getCleanCurrentTree,
+        defaultValue,
+    });
+};

src/hooks/use-async-value.ts

@@ -1,5 +1,5 @@
-import { AsyncResult } from '../types.js';
-import { useEffect, useState } from 'react';
+import { AsyncResult, AsyncResultWithDefault } from '../types.js';
+import { useEffect, useMemo, useState } from 'react';
 
 type State<T> = { type: 'LOADING' } | { type: 'COMPLETE'; value: T };
 
@@ -26,3 +26,21 @@ export const useAsyncValue = <T>({
         isLoading: false,
     };
 };
+
+export const useAsyncValueWithDefault = <T>({
+    getValue,
+    defaultValue,
+}: {
+    getValue: () => Promise<T>;
+    defaultValue: T;
+}): AsyncResultWithDefault<T> => {
+    const result = useAsyncValue({ getValue });
+
+    return useMemo(() => {
+        if (result.isLoading) {
+            return { value: defaultValue, isLoading: true };
+        }
+
+        return { value: result.value, isLoading: false };
+    }, [result.isLoading, result.value, defaultValue]);
+};

src/services/git.ts

@@ -12,7 +12,10 @@ export interface GitService {
     branchLocal: () => Promise<ReturnType<SimpleGit['branchLocal']>>;
     currentBranch: () => Promise<string>;
     listBranches: () => Promise<string[]>;
-    checkout: (branch: string) => Promise<ReturnType<SimpleGit['checkout']>>;
+    checkout: (
+        branch: string,
+        options?: { fallbackBranch?: string }
+    ) => Promise<ReturnType<SimpleGit['checkout']>>;
     addAllFiles: () => Promise<void>;
     commit: (args: { message: string }) => Promise<void>;
     createBranch: (args: { branchName: string }) => Promise<void>;
@@ -32,6 +35,7 @@ export interface GitService {
     fetchPrune: () => Promise<void>;
     pull: () => Promise<void>;
     branchDelete: (branch: string) => Promise<void>;
+    branchExistsLocally: (branch: string) => Promise<boolean>;
 }
 
 export const createGitService = ({
@@ -40,6 +44,16 @@ export const createGitService = ({
     options: Partial<SimpleGitOptions>;
 }): GitService => {
     const gitEngine = simpleGit(options);
+
+    async function branchExistsLocally(branch: string) {
+        // todo: we should probably be using this function a lot more lmao
+        const branchRef = await gitEngine.raw([
+            'show-ref',
+            `refs/heads/${branch}`,
+        ]);
+        return Boolean(branchRef);
+    }
+
     return {
         _git: gitEngine,
         // @ts-expect-error - being weird about the return type
@@ -55,7 +69,17 @@ export const createGitService = ({
             return all;
         },
         // @ts-expect-error - being weird about the return type
-        checkout: async (branch: string) => {
+        checkout: async (
+            branch: string,
+            options?: {
+                fallbackBranch?: string;
+            }
+        ) => {
+            const _branchExistsLocally = await branchExistsLocally(branch);
+            if (!_branchExistsLocally && options?.fallbackBranch) {
+                return gitEngine.checkout(options.fallbackBranch);
+            }
+
             return gitEngine.checkout(branch);
         },
         addAllFiles: async () => {
@@ -89,7 +113,12 @@ export const createGitService = ({
             }
         },
         rebaseContinue: async () => {
-            await gitEngine.rebase(['--continue']);
+            await gitEngine.raw([
+                '-c',
+                'core.editor=true',
+                'rebase',
+                '--continue',
+            ]);
         },
         mergeBaseBranch: async (branchA: string, branchB: string) => {
             const result = await gitEngine.raw([
@@ -120,6 +149,13 @@ export const createGitService = ({
             branch: string;
             ontoBranch: string;
         }) => {
+            if (
+                !(await branchExistsLocally(branch)) ||
+                !(await branchExistsLocally(branch))
+            ) {
+                return false;
+            }
+
             const result = await gitEngine.raw([
                 'merge-base',
                 branch,
@@ -168,5 +204,6 @@ export const createGitService = ({
         branchDelete: async (branch: string) => {
             await gitEngine.deleteLocalBranch(branch, true);
         },
+        branchExistsLocally,
     };
 };

src/services/resolver.ts

@@ -48,7 +48,8 @@ export const recursiveRebase = async ({
         rebasedEventHandler(rebaseAction, 'COMPLETED');
     }
 
-    await git.checkout(endBranch);
+    const rootBranchName = tree.find((b) => b.parent === null)?.key;
+    await git.checkout(endBranch, { fallbackBranch: rootBranchName });
     completeEventHandler();
 };
 

src/services/tree.ts

@@ -67,10 +67,18 @@ const moveOnto = (
 
 const removeBranch = (
     branch: string,
-    deps: { storeService: StoreService; setCurrentTree: SetTreeFunction }
+    deps: { storeService: StoreService; setCurrentTree: SetTreeFunction },
+    options?: { ignoreBranchDoesNotExist?: boolean }
 ): BranchNode | undefined => {
     const tree = _readTree(deps);
-    const branchToRemove = _findBranch({ branch, tree });
+    let branchToRemove: BranchNode;
+
+    try {
+        branchToRemove = _findBranch({ branch, tree });
+    } catch (e) {
+        if (options?.ignoreBranchDoesNotExist) return;
+        throw e;
+    }
 
     if (!branchToRemove) return;
 
@@ -204,7 +212,10 @@ export interface TreeService {
     registerRoot: (branch: string) => void;
     attachTo: (args: { newBranch: string; parent: string }) => void;
     moveOnto: (args: { branch: string; parent: string }) => void;
-    removeBranch: (branch: string) => void;
+    removeBranch: (
+        branch: string,
+        options?: { ignoreBranchDoesNotExist?: boolean }
+    ) => void;
     get: () => Tree;
     getRoot: () => BranchNode | undefined;
     ROOT: symbol;
@@ -229,8 +240,12 @@ export const createTreeService = (config?: TreeServiceConfig): TreeService => {
         moveOnto: (args) => {
             return moveOnto(args, { storeService, setCurrentTree });
         },
-        removeBranch: (branch) => {
-            return removeBranch(branch, { storeService, setCurrentTree });
+        removeBranch: (branch, options) => {
+            return removeBranch(
+                branch,
+                { storeService, setCurrentTree },
+                options
+            );
         },
         get: () => {
             return _readTree({ storeService, setCurrentTree });

src/types.ts

@@ -5,6 +5,8 @@ export type AsyncResult<T> =
     | { value: T; isLoading: false }
     | { value: undefined; isLoading: true };
 
+export type AsyncResultWithDefault<T> = { value: T; isLoading: boolean };
+
 export type SanitizeProps<
     T extends Record<string, unknown>,
     // eslint-disable-next-line  @typescript-eslint/no-explicit-any