refactor: Improve all git operations, add stash support, add improved pull request flow, add worktree file copy options, address code review comments, add cherry pick options

apps/server/tests/unit/routes/worktree/switch-branch.test.ts

@@ -6,7 +6,7 @@ vi.mock('child_process', async (importOriginal) => {
   const actual = await importOriginal<typeof import('child_process')>();
   return {
     ...actual,
-    exec: vi.fn(),
+    execFile: vi.fn(),
   };
 });
 
@@ -18,10 +18,10 @@ vi.mock('util', async (importOriginal) => {
   };
 });
 
-import { exec } from 'child_process';
+import { execFile } from 'child_process';
 import { createSwitchBranchHandler } from '@/routes/worktree/routes/switch-branch.js';
 
-const mockExec = exec as Mock;
+const mockExecFile = execFile as Mock;
 
 describe('switch-branch route', () => {
   let req: Request;
@@ -40,20 +40,21 @@ describe('switch-branch route', () => {
       branchName: 'feature/test',
     };
 
-    mockExec.mockImplementation(async (command: string) => {
+    mockExecFile.mockImplementation(async (file: string, args: string[]) => {
+      const command = `${file} ${args.join(' ')}`;
       if (command === 'git rev-parse --abbrev-ref HEAD') {
         return { stdout: 'main\n', stderr: '' };
       }
-      if (command === 'git rev-parse --verify "feature/test"') {
+      if (command === 'git rev-parse --verify feature/test') {
         return { stdout: 'abc123\n', stderr: '' };
       }
-      if (command === 'git branch -r --format="%(refname:short)"') {
+      if (command === 'git branch -r --format=%(refname:short)') {
         return { stdout: '', stderr: '' };
       }
       if (command === 'git status --porcelain') {
         return { stdout: '?? .automaker/\n?? notes.txt\n', stderr: '' };
       }
-      if (command === 'git checkout "feature/test"') {
+      if (command === 'git checkout feature/test') {
         return { stdout: '', stderr: '' };
       }
       if (command === 'git fetch --all --quiet') {
@@ -84,7 +85,11 @@ describe('switch-branch route', () => {
         stashedChanges: false,
       },
     });
-    expect(mockExec).toHaveBeenCalledWith('git checkout "feature/test"', { cwd: '/repo/path' });
+    expect(mockExecFile).toHaveBeenCalledWith(
+      'git',
+      ['checkout', 'feature/test'],
+      expect.objectContaining({ cwd: '/repo/path' })
+    );
   });
 
   it('should stash changes and switch when tracked files are modified', async () => {
@@ -93,23 +98,25 @@ describe('switch-branch route', () => {
       branchName: 'feature/test',
     };
 
-    mockExec.mockImplementation(async (command: string) => {
+    let stashListCallCount = 0;
+
+    mockExecFile.mockImplementation(async (file: string, args: string[]) => {
+      const command = `${file} ${args.join(' ')}`;
       if (command === 'git rev-parse --abbrev-ref HEAD') {
         return { stdout: 'main\n', stderr: '' };
       }
-      if (command === 'git rev-parse --verify "feature/test"') {
+      if (command === 'git rev-parse --verify feature/test') {
         return { stdout: 'abc123\n', stderr: '' };
       }
       if (command === 'git status --porcelain') {
         return { stdout: ' M src/index.ts\n?? notes.txt\n', stderr: '' };
       }
-      if (command === 'git branch -r --format="%(refname:short)"') {
+      if (command === 'git branch -r --format=%(refname:short)') {
         return { stdout: '', stderr: '' };
       }
       if (command === 'git stash list') {
-        // Return different counts before and after stash to indicate stash was created
-        if (!mockExec._stashCalled) {
-          mockExec._stashCalled = true;
+        stashListCallCount++;
+        if (stashListCallCount === 1) {
           return { stdout: '', stderr: '' };
         }
         return { stdout: 'stash@{0}: automaker-branch-switch\n', stderr: '' };
@@ -117,7 +124,7 @@ describe('switch-branch route', () => {
       if (command.startsWith('git stash push')) {
         return { stdout: '', stderr: '' };
       }
-      if (command === 'git checkout "feature/test"') {
+      if (command === 'git checkout feature/test') {
         return { stdout: '', stderr: '' };
       }
       if (command === 'git fetch --all --quiet') {

apps/server/tests/unit/services/auto-loop-coordinator.test.ts

@@ -6,6 +6,7 @@ import {
   type ProjectAutoLoopState,
   type ExecuteFeatureFn,
   type LoadPendingFeaturesFn,
+  type LoadAllFeaturesFn,
   type SaveExecutionStateFn,
   type ClearExecutionStateFn,
   type ResetStuckFeaturesFn,
@@ -25,6 +26,7 @@ describe('auto-loop-coordinator.ts', () => {
   // Callback mocks
   let mockExecuteFeature: ExecuteFeatureFn;
   let mockLoadPendingFeatures: LoadPendingFeaturesFn;
+  let mockLoadAllFeatures: LoadAllFeaturesFn;
   let mockSaveExecutionState: SaveExecutionStateFn;
   let mockClearExecutionState: ClearExecutionStateFn;
   let mockResetStuckFeatures: ResetStuckFeaturesFn;
@@ -65,6 +67,7 @@ describe('auto-loop-coordinator.ts', () => {
     // Callback mocks
     mockExecuteFeature = vi.fn().mockResolvedValue(undefined);
     mockLoadPendingFeatures = vi.fn().mockResolvedValue([]);
+    mockLoadAllFeatures = vi.fn().mockResolvedValue([]);
     mockSaveExecutionState = vi.fn().mockResolvedValue(undefined);
     mockClearExecutionState = vi.fn().mockResolvedValue(undefined);
     mockResetStuckFeatures = vi.fn().mockResolvedValue(undefined);
@@ -81,7 +84,8 @@ describe('auto-loop-coordinator.ts', () => {
       mockClearExecutionState,
       mockResetStuckFeatures,
       mockIsFeatureFinished,
-      mockIsFeatureRunning
+      mockIsFeatureRunning,
+      mockLoadAllFeatures
     );
   });
 
@@ -326,6 +330,282 @@ describe('auto-loop-coordinator.ts', () => {
     });
   });
 
+  describe('priority-based feature selection', () => {
+    it('selects highest priority feature first (lowest number)', async () => {
+      const lowPriority: Feature = {
+        ...testFeature,
+        id: 'feature-low',
+        priority: 3,
+        title: 'Low Priority',
+      };
+      const highPriority: Feature = {
+        ...testFeature,
+        id: 'feature-high',
+        priority: 1,
+        title: 'High Priority',
+      };
+      const medPriority: Feature = {
+        ...testFeature,
+        id: 'feature-med',
+        priority: 2,
+        title: 'Med Priority',
+      };
+
+      // Return features in non-priority order
+      vi.mocked(mockLoadPendingFeatures).mockResolvedValue([
+        lowPriority,
+        medPriority,
+        highPriority,
+      ]);
+      vi.mocked(mockLoadAllFeatures).mockResolvedValue([lowPriority, medPriority, highPriority]);
+      vi.mocked(mockConcurrencyManager.getRunningCountForWorktree).mockResolvedValue(0);
+
+      await coordinator.startAutoLoopForProject('/test/project', null, 1);
+      await vi.advanceTimersByTimeAsync(3000);
+      await coordinator.stopAutoLoopForProject('/test/project', null);
+
+      // Should execute the highest priority feature (priority=1)
+      expect(mockExecuteFeature).toHaveBeenCalledWith('/test/project', 'feature-high', true, true);
+    });
+
+    it('uses default priority of 2 when not specified', async () => {
+      const noPriority: Feature = { ...testFeature, id: 'feature-none', title: 'No Priority' };
+      const highPriority: Feature = {
+        ...testFeature,
+        id: 'feature-high',
+        priority: 1,
+        title: 'High Priority',
+      };
+
+      vi.mocked(mockLoadPendingFeatures).mockResolvedValue([noPriority, highPriority]);
+      vi.mocked(mockLoadAllFeatures).mockResolvedValue([noPriority, highPriority]);
+      vi.mocked(mockConcurrencyManager.getRunningCountForWorktree).mockResolvedValue(0);
+
+      await coordinator.startAutoLoopForProject('/test/project', null, 1);
+      await vi.advanceTimersByTimeAsync(3000);
+      await coordinator.stopAutoLoopForProject('/test/project', null);
+
+      // High priority (1) should be selected over default priority (2)
+      expect(mockExecuteFeature).toHaveBeenCalledWith('/test/project', 'feature-high', true, true);
+    });
+
+    it('selects first feature when priorities are equal', async () => {
+      const featureA: Feature = {
+        ...testFeature,
+        id: 'feature-a',
+        priority: 2,
+        title: 'Feature A',
+      };
+      const featureB: Feature = {
+        ...testFeature,
+        id: 'feature-b',
+        priority: 2,
+        title: 'Feature B',
+      };
+
+      vi.mocked(mockLoadPendingFeatures).mockResolvedValue([featureA, featureB]);
+      vi.mocked(mockLoadAllFeatures).mockResolvedValue([featureA, featureB]);
+      vi.mocked(mockConcurrencyManager.getRunningCountForWorktree).mockResolvedValue(0);
+
+      await coordinator.startAutoLoopForProject('/test/project', null, 1);
+      await vi.advanceTimersByTimeAsync(3000);
+      await coordinator.stopAutoLoopForProject('/test/project', null);
+
+      // When priorities equal, the first feature from the filtered list should be chosen
+      expect(mockExecuteFeature).toHaveBeenCalledWith('/test/project', 'feature-a', true, true);
+    });
+  });
+
+  describe('dependency-aware feature selection', () => {
+    it('skips features with unsatisfied dependencies', async () => {
+      const depFeature: Feature = {
+        ...testFeature,
+        id: 'feature-dep',
+        status: 'in_progress',
+        title: 'Dependency Feature',
+      };
+      const blockedFeature: Feature = {
+        ...testFeature,
+        id: 'feature-blocked',
+        dependencies: ['feature-dep'],
+        priority: 1,
+        title: 'Blocked Feature',
+      };
+      const readyFeature: Feature = {
+        ...testFeature,
+        id: 'feature-ready',
+        priority: 2,
+        title: 'Ready Feature',
+      };
+
+      // Pending features (backlog/ready status)
+      vi.mocked(mockLoadPendingFeatures).mockResolvedValue([blockedFeature, readyFeature]);
+      // All features (including the in-progress dependency)
+      vi.mocked(mockLoadAllFeatures).mockResolvedValue([depFeature, blockedFeature, readyFeature]);
+      vi.mocked(mockConcurrencyManager.getRunningCountForWorktree).mockResolvedValue(0);
+
+      await coordinator.startAutoLoopForProject('/test/project', null, 1);
+      await vi.advanceTimersByTimeAsync(3000);
+      await coordinator.stopAutoLoopForProject('/test/project', null);
+
+      // Should skip blocked feature (dependency not complete) and execute ready feature
+      expect(mockExecuteFeature).toHaveBeenCalledWith('/test/project', 'feature-ready', true, true);
+      expect(mockExecuteFeature).not.toHaveBeenCalledWith(
+        '/test/project',
+        'feature-blocked',
+        true,
+        true
+      );
+    });
+
+    it('picks features whose dependencies are completed', async () => {
+      const completedDep: Feature = {
+        ...testFeature,
+        id: 'feature-dep',
+        status: 'completed',
+        title: 'Completed Dependency',
+      };
+      const unblockedFeature: Feature = {
+        ...testFeature,
+        id: 'feature-unblocked',
+        dependencies: ['feature-dep'],
+        priority: 1,
+        title: 'Unblocked Feature',
+      };
+
+      vi.mocked(mockLoadPendingFeatures).mockResolvedValue([unblockedFeature]);
+      vi.mocked(mockLoadAllFeatures).mockResolvedValue([completedDep, unblockedFeature]);
+      vi.mocked(mockConcurrencyManager.getRunningCountForWorktree).mockResolvedValue(0);
+
+      await coordinator.startAutoLoopForProject('/test/project', null, 1);
+      await vi.advanceTimersByTimeAsync(3000);
+      await coordinator.stopAutoLoopForProject('/test/project', null);
+
+      // Should execute the unblocked feature since its dependency is completed
+      expect(mockExecuteFeature).toHaveBeenCalledWith(
+        '/test/project',
+        'feature-unblocked',
+        true,
+        true
+      );
+    });
+
+    it('picks features whose dependencies are verified', async () => {
+      const verifiedDep: Feature = {
+        ...testFeature,
+        id: 'feature-dep',
+        status: 'verified',
+        title: 'Verified Dependency',
+      };
+      const unblockedFeature: Feature = {
+        ...testFeature,
+        id: 'feature-unblocked',
+        dependencies: ['feature-dep'],
+        priority: 1,
+        title: 'Unblocked Feature',
+      };
+
+      vi.mocked(mockLoadPendingFeatures).mockResolvedValue([unblockedFeature]);
+      vi.mocked(mockLoadAllFeatures).mockResolvedValue([verifiedDep, unblockedFeature]);
+      vi.mocked(mockConcurrencyManager.getRunningCountForWorktree).mockResolvedValue(0);
+
+      await coordinator.startAutoLoopForProject('/test/project', null, 1);
+      await vi.advanceTimersByTimeAsync(3000);
+      await coordinator.stopAutoLoopForProject('/test/project', null);
+
+      expect(mockExecuteFeature).toHaveBeenCalledWith(
+        '/test/project',
+        'feature-unblocked',
+        true,
+        true
+      );
+    });
+
+    it('respects both priority and dependencies together', async () => {
+      const completedDep: Feature = {
+        ...testFeature,
+        id: 'feature-dep',
+        status: 'completed',
+        title: 'Completed Dep',
+      };
+      const blockedHighPriority: Feature = {
+        ...testFeature,
+        id: 'feature-blocked-hp',
+        dependencies: ['feature-not-done'],
+        priority: 1,
+        title: 'Blocked High Priority',
+      };
+      const unblockedLowPriority: Feature = {
+        ...testFeature,
+        id: 'feature-unblocked-lp',
+        dependencies: ['feature-dep'],
+        priority: 3,
+        title: 'Unblocked Low Priority',
+      };
+      const unblockedMedPriority: Feature = {
+        ...testFeature,
+        id: 'feature-unblocked-mp',
+        priority: 2,
+        title: 'Unblocked Med Priority',
+      };
+
+      vi.mocked(mockLoadPendingFeatures).mockResolvedValue([
+        blockedHighPriority,
+        unblockedLowPriority,
+        unblockedMedPriority,
+      ]);
+      vi.mocked(mockLoadAllFeatures).mockResolvedValue([
+        completedDep,
+        blockedHighPriority,
+        unblockedLowPriority,
+        unblockedMedPriority,
+      ]);
+      vi.mocked(mockConcurrencyManager.getRunningCountForWorktree).mockResolvedValue(0);
+
+      await coordinator.startAutoLoopForProject('/test/project', null, 1);
+      await vi.advanceTimersByTimeAsync(3000);
+      await coordinator.stopAutoLoopForProject('/test/project', null);
+
+      // Should skip blocked high-priority and pick the unblocked medium-priority
+      expect(mockExecuteFeature).toHaveBeenCalledWith(
+        '/test/project',
+        'feature-unblocked-mp',
+        true,
+        true
+      );
+      expect(mockExecuteFeature).not.toHaveBeenCalledWith(
+        '/test/project',
+        'feature-blocked-hp',
+        true,
+        true
+      );
+    });
+
+    it('handles features with no dependencies (always eligible)', async () => {
+      const noDeps: Feature = {
+        ...testFeature,
+        id: 'feature-no-deps',
+        priority: 2,
+        title: 'No Dependencies',
+      };
+
+      vi.mocked(mockLoadPendingFeatures).mockResolvedValue([noDeps]);
+      vi.mocked(mockLoadAllFeatures).mockResolvedValue([noDeps]);
+      vi.mocked(mockConcurrencyManager.getRunningCountForWorktree).mockResolvedValue(0);
+
+      await coordinator.startAutoLoopForProject('/test/project', null, 1);
+      await vi.advanceTimersByTimeAsync(3000);
+      await coordinator.stopAutoLoopForProject('/test/project', null);
+
+      expect(mockExecuteFeature).toHaveBeenCalledWith(
+        '/test/project',
+        'feature-no-deps',
+        true,
+        true
+      );
+    });
+  });
+
   describe('failure tracking', () => {
     it('trackFailureAndCheckPauseForProject returns true after threshold', async () => {
       await coordinator.startAutoLoopForProject('/test/project', null, 1);