diff --git a/backend/src/routes/pools.ts b/backend/src/routes/pools.ts index b58fd04..90c46d3 100644 --- a/backend/src/routes/pools.ts +++ b/backend/src/routes/pools.ts @@ -5,10 +5,12 @@ import { Router, Request, Response } from 'express'; import { v4 as uuidv4 } from 'uuid'; -import { db, type RunnerPoolRow, type CredentialRow } from '../db/index.js'; +import { db, type RunnerPoolRow, type CredentialRow, type RunnerRow } from '../db/index.js'; import { decrypt, generateSecret } from '../utils/index.js'; import { createGitHubClient, type GitHubScope } from '../services/index.js'; import { ensureWarmRunners, getPoolById as getPoolRowById } from '../services/autoscaler.js'; +import { removeRunner, stopOrphanedRunner, cleanupRunnerFiles } from '../services/runnerManager.js'; +import { removeDockerRunner } from '../services/dockerRunner.js'; export const poolsRouter = Router(); @@ -81,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(` @@ -299,7 +303,7 @@ poolsRouter.patch('/:id', async (req: Request, res: Response) => { }); /** - * Delete a pool + * Delete a pool and all its runners */ poolsRouter.delete('/:id', async (req: Request, res: Response) => { try { @@ -310,9 +314,54 @@ poolsRouter.delete('/:id', async (req: Request, res: Response) => { return; } - // Note: Runners belonging to this pool will have pool_id set to NULL - // They won't be deleted automatically + // 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`); + + // Clean up each runner in parallel (fire-and-forget pattern for speed) + const cleanupPromises = poolRunners.map(async (runner) => { + try { + console.log(`[pools] Cleaning up runner ${runner.name} (${runner.id})`); + + if (runner.isolation_type === 'docker') { + await removeDockerRunner(runner.id).catch((err) => { + console.error(`[pools] Failed to remove Docker runner ${runner.id}:`, err); + }); + } else { + // For native runners, try the full removal first + // This handles deregistration from GitHub and cleanup + await removeRunner(runner.id).catch(async (err) => { + // If removeRunner fails (e.g., runner already gone), do manual cleanup + console.error(`[pools] removeRunner failed for ${runner.id}, doing manual cleanup:`, err); + await stopOrphanedRunner(runner.id, runner.process_id).catch(() => {}); + await cleanupRunnerFiles(runner.runner_dir).catch(() => {}); + }); + } + + // Delete the database record + deleteRunnerById.run(runner.id); + console.log(`[pools] Cleaned up runner ${runner.name}`); + } catch (err) { + console.error(`[pools] Failed to cleanup runner ${runner.name}:`, err); + // Still try to delete the database record + try { + deleteRunnerById.run(runner.id); + } catch (deleteErr) { + console.error(`[pools] Failed to delete runner record ${runner.id} during cleanup:`, deleteErr); + } + } + }); + + // Wait for all cleanup to complete (with a timeout to avoid hanging) + await Promise.allSettled(cleanupPromises); + + // Now delete the pool deletePoolById.run(req.params.id); + console.log(`[pools] Deleted pool ${req.params.id}`); res.status(204).send(); } catch (error) { diff --git a/backend/tests/pools.test.ts b/backend/tests/pools.test.ts index 8fe2527..ed86e96 100644 --- a/backend/tests/pools.test.ts +++ b/backend/tests/pools.test.ts @@ -195,5 +195,50 @@ describe('Pools API', () => { const getResponse = await request(app).get(`/api/pools/${id}`); expect(getResponse.status).toBe(404); }); + + it('should clean up associated runners when deleting a pool', async () => { + // Create a pool first + const createResponse = await request(app) + .post('/api/pools') + .send({ + name: 'test-pool-with-runners', + credentialId: credentialId, + }); + + 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(); + + db.prepare(` + INSERT INTO runners (id, name, status, pool_id, credential_id, isolation_type, ephemeral, platform, architecture, labels) + VALUES (?, ?, 'offline', ?, ?, 'native', 0, 'linux', 'x64', '[]') + `).run(runnerId1, 'test-runner-1', poolId, credentialId); + + db.prepare(` + INSERT INTO runners (id, name, status, pool_id, credential_id, isolation_type, ephemeral, platform, architecture, labels) + VALUES (?, ?, 'offline', ?, ?, 'native', 0, 'linux', 'x64', '[]') + `).run(runnerId2, 'test-runner-2', poolId, credentialId); + + // Verify runners exist + const runnersBefore = db.prepare('SELECT id FROM runners WHERE pool_id = ?').all(poolId); + expect(runnersBefore.length).toBe(2); + + // Delete the pool + const response = await request(app).delete(`/api/pools/${poolId}`); + expect(response.status).toBe(204); + + // Verify the pool is deleted + const getPoolResponse = await request(app).get(`/api/pools/${poolId}`); + expect(getPoolResponse.status).toBe(404); + + // Verify runners are also deleted (not just orphaned with NULL pool_id) + const runnersAfter = db.prepare('SELECT id FROM runners WHERE id IN (?, ?)').all(runnerId1, runnerId2); + expect(runnersAfter.length).toBe(0); + }); }); }); diff --git a/frontend/package.json b/frontend/package.json index 43d824b..d43dce1 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -8,7 +8,7 @@ "build": "tsc -b && vite build", "lint": "eslint .", "preview": "vite preview", - "test": "vitest", + "test": "vitest run", "typecheck": "tsc -b --noEmit" }, "dependencies": { diff --git a/frontend/src/components/RunnerManager.tsx b/frontend/src/components/RunnerManager.tsx index 8fd1cf9..f297803 100644 --- a/frontend/src/components/RunnerManager.tsx +++ b/frontend/src/components/RunnerManager.tsx @@ -232,6 +232,8 @@ function AddRunnerForm({ function RunnerRow({ runner, + selected, + onSelect, onStart, onStop, onSync, @@ -239,12 +241,16 @@ function RunnerRow({ onViewLogs, }: { runner: Runner; + selected: boolean; + onSelect: (id: string, selected: boolean) => void; onStart: (id: string) => void; onStop: (id: string) => void; onSync: (id: string) => void; onDelete: (id: string) => void; onViewLogs: (runner: Runner) => void; }) { + const isOrphaned = runner.ephemeral && !runner.pool_id; + const platformIcons: Record = { darwin: '🍎', linux: '🐧', @@ -264,12 +270,27 @@ function RunnerRow({ const canViewLogs = runner.isolation_type === 'docker' || runner.runner_dir; return ( - + + + onSelect(runner.id, e.target.checked)} + className="w-4 h-4 rounded border-forest-600 bg-forest-800 text-forest-400 focus:ring-forest-500" + /> +
{platformIcons[effectivePlatform] || '💻'}
-
{runner.name}
+
+ {runner.name} + {isOrphaned && ( + + orphaned + + )} +
{runner.target}
@@ -363,6 +384,8 @@ function RunnerRow({ export function RunnerManager() { const [showAddForm, setShowAddForm] = useState(false); const [selectedRunnerForLogs, setSelectedRunnerForLogs] = useState(null); + const [selectedRunnerIds, setSelectedRunnerIds] = useState>(new Set()); + const [showOrphanedOnly, setShowOrphanedOnly] = useState(false); const queryClient = useQueryClient(); const { data: runnersData, isLoading: runnersLoading } = useQuery({ @@ -398,7 +421,10 @@ export function RunnerManager() { const deleteMutation = useMutation({ mutationFn: runnersApi.delete, - onSuccess: () => queryClient.invalidateQueries({ queryKey: ['runners'] }), + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: ['runners'] }); + setSelectedRunnerIds(new Set()); + }, }); const handleDelete = (id: string) => { @@ -410,6 +436,71 @@ export function RunnerManager() { const runners = runnersData?.runners || []; const credentials = credentialsData?.credentials || []; + // Filter runners based on orphaned filter + const filteredRunners = showOrphanedOnly + ? runners.filter(r => r.ephemeral && !r.pool_id) + : runners; + + const orphanedCount = runners.filter(r => r.ephemeral && !r.pool_id).length; + + // Selection helpers + const handleSelectRunner = (id: string, selected: boolean) => { + setSelectedRunnerIds(prev => { + const next = new Set(prev); + if (selected) { + next.add(id); + } else { + next.delete(id); + } + return next; + }); + }; + + const handleSelectAll = (selected: boolean) => { + if (selected) { + setSelectedRunnerIds(new Set(filteredRunners.map(r => r.id))); + } else { + setSelectedRunnerIds(new Set()); + } + }; + + const handleBulkDelete = async () => { + const count = selectedRunnerIds.size; + if (count === 0) return; + + if (!confirm(`Are you sure you want to delete ${count} runner${count === 1 ? '' : 's'}? This will stop the runners and deregister them from GitHub.`)) { + return; + } + + // 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()); + queryClient.invalidateQueries({ queryKey: ['runners'] }); + }; + + const allFilteredSelected = filteredRunners.length > 0 && filteredRunners.every(r => selectedRunnerIds.has(r.id)); + const someFilteredSelected = filteredRunners.some(r => selectedRunnerIds.has(r.id)); + return (
{/* Header */} @@ -451,23 +542,76 @@ export function RunnerManager() {

) : ( -
- - - - - - - - + <> + {/* Filter and bulk actions bar */} +
+
+ + {selectedRunnerIds.size > 0 && ( + + {selectedRunnerIds.size} selected + + )} +
+ {selectedRunnerIds.size > 0 && ( + + )} +
+ +
+
RunnerStatusOS/ArchIsolationLabels
+ + + + + + + + - {runners.map((runner) => ( + {filteredRunners.map((runner) => (
+ { + if (el) el.indeterminate = someFilteredSelected && !allFilteredSelected; + }} + onChange={(e) => handleSelectAll(e.target.checked)} + className="w-4 h-4 rounded border-forest-600 bg-forest-800 text-forest-400 focus:ring-forest-500" + /> + RunnerStatusOS/ArchIsolationLabels Actions
+ )} {/* Add Form Modal */}