From fc5851e3a24a30437a07ed82546a61186ac56379 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Fri, 22 May 2026 13:37:21 -0400 Subject: [PATCH] fix: ensure dashboard cliproxy restart waits for recovery --- .../routes/cliproxy-stats-routes.ts | 45 +++----- .../cliproxy-dashboard-restart-service.ts | 79 +++++++++++++ ...cliproxy-dashboard-restart-service.test.ts | 104 ++++++++++++++++++ .../cliproxy-stats-routes-restart.test.ts | 46 ++++++++ 4 files changed, 244 insertions(+), 30 deletions(-) create mode 100644 src/web-server/services/cliproxy-dashboard-restart-service.ts create mode 100644 tests/unit/web-server/cliproxy-dashboard-restart-service.test.ts create mode 100644 tests/unit/web-server/cliproxy-stats-routes-restart.test.ts diff --git a/src/web-server/routes/cliproxy-stats-routes.ts b/src/web-server/routes/cliproxy-stats-routes.ts index 64d3d3e1..6469262e 100644 --- a/src/web-server/routes/cliproxy-stats-routes.ts +++ b/src/web-server/routes/cliproxy-stats-routes.ts @@ -5,10 +5,6 @@ import { Router, Request, Response } from 'express'; import * as fs from 'fs'; import * as path from 'path'; -import { - isRunningUnderSupervisord, - restartCliproxyViaSupervisord, -} from '../../docker/supervisord-lifecycle'; import { fetchCliproxyStats, fetchCliproxyModels, @@ -59,9 +55,11 @@ import { getDeniedModelIdReasonForProvider, } from '../../cliproxy/ai-providers/model-id-normalizer'; import { installDashboardCliproxyVersion } from '../services/cliproxy-dashboard-install-service'; +import { restartDashboardCliproxy } from '../services/cliproxy-dashboard-restart-service'; import { requireLocalAccessWhenAuthDisabled } from '../middleware/auth-middleware'; const router = Router(); +type RestartDashboardCliproxyHandler = typeof restartDashboardCliproxy; const QUOTA_RATE_LIMIT_WINDOW_MS = 60_000; const QUOTA_RATE_LIMIT_MAX_REQUESTS = 120; @@ -1085,34 +1083,21 @@ router.post('/install', async (req: Request, res: Response): Promise => { * POST /api/cliproxy/restart - Restart CLIProxy without version change * Returns: { success, port?, error? } */ -router.post('/restart', async (_req: Request, res: Response): Promise => { - try { - const port = resolveLifecyclePort(); - if (isRunningUnderSupervisord()) { - // Docker mode: delegate to supervisord which owns the process lifecycle - const result = restartCliproxyViaSupervisord(); +export function registerCliproxyRestartRoute( + targetRouter: Router, + restartHandler: RestartDashboardCliproxyHandler = restartDashboardCliproxy +): void { + targetRouter.post('/restart', async (_req: Request, res: Response): Promise => { + try { + const result = await restartHandler(); res.json(result); - return; + } catch (error) { + console.error(`[cliproxy-stats] ${(error as Error).message}`); + res.status(500).json({ error: 'Internal server error' }); } + }); +} - // Local mode: direct process management - await stopProxy(port); - - // Small delay to ensure port is released - await new Promise((r) => setTimeout(r, 500)); - - // Start proxy - const startResult = await ensureCliproxyService(port); - - if (startResult.started || startResult.alreadyRunning) { - res.json({ success: true, port: startResult.port }); - } else { - res.json({ success: false, error: startResult.error || 'Failed to start proxy' }); - } - } catch (error) { - console.error(`[cliproxy-stats] ${(error as Error).message}`); - res.status(500).json({ error: 'Internal server error' }); - } -}); +registerCliproxyRestartRoute(router); export default router; diff --git a/src/web-server/services/cliproxy-dashboard-restart-service.ts b/src/web-server/services/cliproxy-dashboard-restart-service.ts new file mode 100644 index 00000000..2a6cd5ac --- /dev/null +++ b/src/web-server/services/cliproxy-dashboard-restart-service.ts @@ -0,0 +1,79 @@ +import { resolveLifecyclePort } from '../../cliproxy/config/port-manager'; +import { ensureCliproxyService, type ServiceStartResult } from '../../cliproxy/service-manager'; +import { waitForProxyHealthy } from '../../cliproxy/proxy/proxy-detector'; +import { stopProxy } from '../../cliproxy/session-tracker'; +import { + isRunningUnderSupervisord, + restartCliproxyViaSupervisord, +} from '../../docker/supervisord-lifecycle'; + +export interface CliproxyDashboardRestartResult { + success: boolean; + port?: number; + error?: string; +} + +export interface CliproxyDashboardRestartDeps { + resolveLifecyclePortFn?: typeof resolveLifecyclePort; + isRunningUnderSupervisordFn?: typeof isRunningUnderSupervisord; + restartCliproxyViaSupervisordFn?: typeof restartCliproxyViaSupervisord; + waitForProxyHealthyFn?: typeof waitForProxyHealthy; + stopProxyFn?: typeof stopProxy; + ensureCliproxyServiceFn?: typeof ensureCliproxyService; + delayFn?: (ms: number) => Promise; +} + +const RESTART_SETTLE_MS = 500; +const SUPERVISOR_HEALTH_TIMEOUT_MS = 10_000; + +function delay(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +function startResultToRestartResult( + startResult: ServiceStartResult +): CliproxyDashboardRestartResult { + if (startResult.started || startResult.alreadyRunning) { + return { success: true, port: startResult.port }; + } + + return { success: false, error: startResult.error || 'Failed to start proxy' }; +} + +export async function restartDashboardCliproxy( + deps: CliproxyDashboardRestartDeps = {} +): Promise { + const resolveLifecyclePortFn = deps.resolveLifecyclePortFn ?? resolveLifecyclePort; + const isRunningUnderSupervisordFn = deps.isRunningUnderSupervisordFn ?? isRunningUnderSupervisord; + const restartCliproxyViaSupervisordFn = + deps.restartCliproxyViaSupervisordFn ?? restartCliproxyViaSupervisord; + const waitForProxyHealthyFn = deps.waitForProxyHealthyFn ?? waitForProxyHealthy; + const stopProxyFn = deps.stopProxyFn ?? stopProxy; + const ensureCliproxyServiceFn = deps.ensureCliproxyServiceFn ?? ensureCliproxyService; + const delayFn = deps.delayFn ?? delay; + + const port = resolveLifecyclePortFn(); + + if (isRunningUnderSupervisordFn()) { + const supervisorResult = restartCliproxyViaSupervisordFn(); + if (!supervisorResult.success) { + return supervisorResult; + } + + const healthy = await waitForProxyHealthyFn(port, SUPERVISOR_HEALTH_TIMEOUT_MS); + if (!healthy) { + return { + success: false, + error: 'CLIProxy did not become healthy after supervisor restart', + }; + } + + return { success: true, port: supervisorResult.port ?? port }; + } + + await stopProxyFn(port); + await delayFn(RESTART_SETTLE_MS); + + const startResult = await ensureCliproxyServiceFn(port); + return startResultToRestartResult(startResult); +} diff --git a/tests/unit/web-server/cliproxy-dashboard-restart-service.test.ts b/tests/unit/web-server/cliproxy-dashboard-restart-service.test.ts new file mode 100644 index 00000000..eb912acf --- /dev/null +++ b/tests/unit/web-server/cliproxy-dashboard-restart-service.test.ts @@ -0,0 +1,104 @@ +import { describe, expect, it } from 'bun:test'; + +import { + restartDashboardCliproxy, + type CliproxyDashboardRestartDeps, +} from '../../../src/web-server/services/cliproxy-dashboard-restart-service'; + +function createDeps(overrides: Partial = {}) { + const calls: string[] = []; + const deps: CliproxyDashboardRestartDeps = { + resolveLifecyclePortFn: () => 8317, + isRunningUnderSupervisordFn: () => false, + restartCliproxyViaSupervisordFn: () => { + calls.push('supervisor-restart'); + return { success: true, port: 8317 }; + }, + waitForProxyHealthyFn: async () => { + calls.push('wait-healthy'); + return true; + }, + stopProxyFn: async () => { + calls.push('stop'); + return { stopped: true, port: 8317, pid: 1234, sessionCount: 1 }; + }, + ensureCliproxyServiceFn: async () => { + calls.push('start'); + return { started: true, alreadyRunning: false, port: 8317 }; + }, + delayFn: async () => { + calls.push('delay'); + }, + ...overrides, + }; + + return { calls, deps }; +} + +describe('cliproxy dashboard restart service', () => { + it('performs a local restart as stop then start, not stop only', async () => { + const { calls, deps } = createDeps(); + + const result = await restartDashboardCliproxy(deps); + + expect(result).toEqual({ success: true, port: 8317 }); + expect(calls).toEqual(['stop', 'delay', 'start']); + }); + + it('starts a local instance even when stop finds no active session', async () => { + const { calls, deps } = createDeps({ + stopProxyFn: async () => { + calls.push('stop'); + return { stopped: false, port: 8317, error: 'No active CLIProxy session found' }; + }, + }); + + const result = await restartDashboardCliproxy(deps); + + expect(result).toEqual({ success: true, port: 8317 }); + expect(calls).toEqual(['stop', 'delay', 'start']); + }); + + it('returns failure when local restart cannot start after stopping', async () => { + const { calls, deps } = createDeps({ + ensureCliproxyServiceFn: async () => { + calls.push('start'); + return { started: false, alreadyRunning: false, port: 8317, error: 'port blocked' }; + }, + }); + + const result = await restartDashboardCliproxy(deps); + + expect(result).toEqual({ success: false, error: 'port blocked' }); + expect(calls).toEqual(['stop', 'delay', 'start']); + }); + + it('waits for Docker supervisor restart to become healthy before reporting success', async () => { + const { calls, deps } = createDeps({ + isRunningUnderSupervisordFn: () => true, + }); + + const result = await restartDashboardCliproxy(deps); + + expect(result).toEqual({ success: true, port: 8317 }); + expect(calls).toEqual(['supervisor-restart', 'wait-healthy']); + }); + + it('reports Docker supervisor restart as failed when the proxy never becomes healthy', async () => { + const { calls, deps } = createDeps({ + isRunningUnderSupervisordFn: () => true, + waitForProxyHealthyFn: async () => { + calls.push('wait-healthy'); + return false; + }, + }); + + const result = await restartDashboardCliproxy(deps); + + expect(result).toEqual({ + success: false, + error: 'CLIProxy did not become healthy after supervisor restart', + }); + expect(calls).toEqual(['supervisor-restart', 'wait-healthy']); + }); +}); diff --git a/tests/unit/web-server/cliproxy-stats-routes-restart.test.ts b/tests/unit/web-server/cliproxy-stats-routes-restart.test.ts new file mode 100644 index 00000000..80c68600 --- /dev/null +++ b/tests/unit/web-server/cliproxy-stats-routes-restart.test.ts @@ -0,0 +1,46 @@ +import { afterEach, beforeEach, describe, expect, it, mock } from 'bun:test'; +import express from 'express'; +import type { Server } from 'http'; +import { registerCliproxyRestartRoute } from '../../../src/web-server/routes/cliproxy-stats-routes'; + +describe('cliproxy stats routes restart endpoint', () => { + let server: Server; + let baseUrl = ''; + let restartMock: ReturnType; + + beforeEach(async () => { + restartMock = mock(async () => ({ success: true, port: 8317 })); + + const app = express(); + app.use(express.json()); + const restartRouter = express.Router(); + registerCliproxyRestartRoute(restartRouter, restartMock); + app.use('/api/cliproxy', restartRouter); + + server = await new Promise((resolve, reject) => { + const instance = app.listen(0, '127.0.0.1'); + instance.once('error', reject); + instance.once('listening', () => resolve(instance)); + }); + + const address = server.address(); + if (!address || typeof address === 'string') { + throw new Error('Unable to resolve test server port'); + } + baseUrl = `http://127.0.0.1:${address.port}`; + }); + + afterEach(async () => { + if (server) { + await new Promise((resolve) => server.close(() => resolve())); + } + }); + + it('routes POST /api/cliproxy/restart through restart semantics', async () => { + const response = await fetch(`${baseUrl}/api/cliproxy/restart`, { method: 'POST' }); + + expect(response.status).toBe(200); + expect(await response.json()).toEqual({ success: true, port: 8317 }); + expect(restartMock).toHaveBeenCalledTimes(1); + }); +});