Address PR feedback from Copilot review

- Move deleteRunnerById prepared statement to top with other statements
- Fix misleading comment about ON DELETE SET NULL behavior
- Add race condition acknowledgment comment with note about orphan detection
- Use Promise.allSettled for parallel bulk deletion in frontend
- Add user notification when bulk delete has failures
- Add comment explaining test runners are simplified without directories

Author: depoll

backend/src/routes/pools.ts

@@ -83,6 +83,8 @@ const updatePoolEnabled = db.prepare(`
 
 const deletePoolById = db.prepare('DELETE FROM runner_pools WHERE id = ?');
 
+const deleteRunnerById = db.prepare('DELETE FROM runners WHERE id = ?');
+
 const getCredentialById = db.prepare('SELECT * FROM credentials WHERE id = ?');
 
 const getPoolRunners = db.prepare(`
@@ -300,9 +302,6 @@ poolsRouter.patch('/:id', async (req: Request, res: Response) => {
   }
 });
 
-// Prepared statement for deleting a runner from the database
-const deleteRunnerById = db.prepare('DELETE FROM runners WHERE id = ?');
-
 /**
  * Delete a pool and all its runners
  */
@@ -316,6 +315,9 @@ poolsRouter.delete('/:id', async (req: Request, res: Response) => {
     }
 
     // Get all runners belonging to this pool and clean them up
+    // Note: There's a small race window where a new runner could be added between this query
+    // and pool deletion. Such runners would have pool_id set to NULL by ON DELETE SET NULL.
+    // The UI's orphan detection feature can help identify and clean up such cases if they occur.
     const poolRunners = getPoolRunners.all(req.params.id) as RunnerRow[];
 
     console.log(`[pools] Deleting pool ${req.params.id} with ${poolRunners.length} runners`);
@@ -348,8 +350,8 @@ poolsRouter.delete('/:id', async (req: Request, res: Response) => {
         // Still try to delete the database record
         try {
           deleteRunnerById.run(runner.id);
-        } catch {
-          // Ignore - may already be deleted by ON DELETE SET NULL behavior
+        } catch (deleteErr) {
+          console.error(`[pools] Failed to delete runner record ${runner.id} during cleanup:`, deleteErr);
         }
       }
     });

backend/tests/pools.test.ts

@@ -208,6 +208,9 @@ describe('Pools API', () => {
       const poolId = createResponse.body.pool.id;
 
       // Insert test runners directly into the database (simulating runners that exist)
+      // Note: These are simplified test runners without runner_dir or process_id.
+      // The cleanup code handles this gracefully - removeRunner will fail but the
+      // fallback cleanup and database deletion will still succeed.
       const runnerId1 = 'test-runner-1-' + Date.now();
       const runnerId2 = 'test-runner-2-' + Date.now();
 

frontend/src/components/RunnerManager.tsx

@@ -472,13 +472,26 @@ export function RunnerManager() {
       return;
     }
 
-    // Delete runners one by one (could be optimized with a bulk API endpoint)
-    for (const id of selectedRunnerIds) {
-      try {
-        await runnersApi.delete(id);
-      } catch (err) {
-        console.error(`Failed to delete runner ${id}:`, err);
+    // Delete runners in parallel (could be further optimized with a bulk API endpoint)
+    const ids = Array.from(selectedRunnerIds);
+    const results = await Promise.allSettled(
+      ids.map(id => runnersApi.delete(id))
+    );
+    
+    // Collect and report failures
+    const failedIds: string[] = [];
+    results.forEach((result, index) => {
+      if (result.status === 'rejected') {
+        console.error(`Failed to delete runner ${ids[index]}:`, result.reason);
+        failedIds.push(ids[index]);
       }
+    });
+    
+    if (failedIds.length > 0) {
+      alert(
+        `Failed to delete ${failedIds.length} runner${failedIds.length === 1 ? '' : 's'}. ` +
+        'Please check the console for more details and try again.'
+      );
     }
 
     setSelectedRunnerIds(new Set());