From bd48ffacd8f607065afd9830e1bcddbd5cd0f491 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Fri, 28 Aug 2026 21:31:35 +0000 Subject: [PATCH] refactor(dev): fold lifecycle shutdown into one ordered close loop The two cleanup recommendations from the Aug 18 simplification review that were never applied: closeDevServerLifecycle's six sequential Promise.allSettled blocks and per-resource failure spreads collapse into one ordered producer table and close loop with identical close ordering and failure semantics, and the Workbench root hoists the foreground client once instead of asserting foreground.current! at eight construction sites. --- .../agent-bundle/src/dev/workbench-server.ts | 76 ++++++------------- packages/workbench/src/main.tsx | 21 ++--- 2 files changed, 33 insertions(+), 64 deletions(-) diff --git a/packages/agent-bundle/src/dev/workbench-server.ts b/packages/agent-bundle/src/dev/workbench-server.ts index b3ec400f4..9584368c7 100644 --- a/packages/agent-bundle/src/dev/workbench-server.ts +++ b/packages/agent-bundle/src/dev/workbench-server.ts @@ -419,66 +419,34 @@ export const closeDevServerLifecycle = async ({ producer: 'project', summary: 'Development workbench shutdown started.', }); - const playgroundResults = playground === undefined ? [] : await Promise.allSettled([playground.close()]); - const appResults = mcpApps === undefined ? [] : await Promise.allSettled([mcpApps.close()]); - const clientSurfaceResults = runtimeResources?.clientSurfaces === undefined - ? [] - : await Promise.allSettled([runtimeResources.clientSurfaces.close()]); - const runtimeResults = runtimeResources?.runtime === undefined - ? [] - : await Promise.allSettled([runtimeResources.runtime.close()]); - const sessionResults = await Promise.allSettled([mcpSessions.close()]); - const coordinatorResults = await Promise.allSettled([coordinator.close()]); + // Producers close strictly in this order, each awaited before the next; + // the ordering is load-bearing for the sessions the coordinator drains. + const producers: readonly (readonly [DevServerLifecycleCloseFailure['resource'], Closeable | undefined])[] = [ + ['playground', playground], + ['mcp-apps', mcpApps], + ['runtime-client-surfaces', runtimeResources?.clientSurfaces], + ['runtime', runtimeResources?.runtime], + ['mcp-sessions', mcpSessions], + ['coordinator', coordinator], + ]; + const failures: DevServerLifecycleCloseFailure[] = []; + const closeResource = async (resource: DevServerLifecycleCloseFailure['resource'], closeable: Closeable): Promise => { + const [result] = await Promise.allSettled([closeable.close()]); + if (result?.status === 'rejected') failures.push(Object.freeze({ error: result.reason, resource })); + }; + for (const [resource, closeable] of producers) { + if (closeable !== undefined) await closeResource(resource, closeable); + } try { detachProjectLogs?.(); } catch { /* The subscription is observability-only and cannot hold shutdown. */ } - const producerFailures = [ - ...playgroundResults.flatMap((result): readonly DevServerLifecycleCloseFailure[] => - result.status === 'rejected' - ? [Object.freeze({ error: result.reason, resource: 'playground' as const })] - : [], - ), - ...appResults.flatMap((result): readonly DevServerLifecycleCloseFailure[] => - result.status === 'rejected' - ? [Object.freeze({ error: result.reason, resource: 'mcp-apps' as const })] - : [], - ), - ...clientSurfaceResults.flatMap((result): readonly DevServerLifecycleCloseFailure[] => - result.status === 'rejected' - ? [Object.freeze({ error: result.reason, resource: 'runtime-client-surfaces' as const })] - : [], - ), - ...runtimeResults.flatMap((result): readonly DevServerLifecycleCloseFailure[] => - result.status === 'rejected' - ? [Object.freeze({ error: result.reason, resource: 'runtime' as const })] - : [], - ), - ...sessionResults.flatMap((result): readonly DevServerLifecycleCloseFailure[] => - result.status === 'rejected' - ? [Object.freeze({ error: result.reason, resource: 'mcp-sessions' as const })] - : [], - ), - ...coordinatorResults.flatMap((result): readonly DevServerLifecycleCloseFailure[] => - result.status === 'rejected' - ? [Object.freeze({ error: result.reason, resource: 'coordinator' as const })] - : [], - ), - ]; logs?.log({ - details: { failures: producerFailures.length }, + details: { failures: failures.length }, kind: 'dev.shutdown.completed', - level: producerFailures.length === 0 ? 'info' : 'warning', + level: failures.length === 0 ? 'info' : 'warning', producer: 'project', - summary: producerFailures.length === 0 ? 'Development workbench shutdown completed.' : 'Development workbench shutdown completed with failures.', + summary: failures.length === 0 ? 'Development workbench shutdown completed.' : 'Development workbench shutdown completed with failures.', }); - const logsResults = logs === undefined ? [] : await Promise.allSettled([logs.close()]); - const failures = [ - ...producerFailures, - ...logsResults.flatMap((result): readonly DevServerLifecycleCloseFailure[] => - result.status === 'rejected' - ? [Object.freeze({ error: result.reason, resource: 'logs' as const })] - : [], - ), - ]; + if (logs !== undefined) await closeResource('logs', logs); if (failures.length > 0) throw new DevServerLifecycleCloseError(failures); }; diff --git a/packages/workbench/src/main.tsx b/packages/workbench/src/main.tsx index 969c8020e..775dcf964 100644 --- a/packages/workbench/src/main.tsx +++ b/packages/workbench/src/main.tsx @@ -852,7 +852,8 @@ const Workbench = () => { const [capabilityRetry, setCapabilityRetry] = useState(0); const [capabilityState, setCapabilityState] = useState({ state: 'empty' }); if (foreground.current === undefined) foreground.current = new ForegroundRouteClient(); - if (mcpRoutes.current === undefined) mcpRoutes.current = new WorkbenchMcpRouteClient({ foreground: foreground.current }); + const foregroundClient = foreground.current; + if (mcpRoutes.current === undefined) mcpRoutes.current = new WorkbenchMcpRouteClient({ foreground: foregroundClient }); const [mcpController, setMcpController] = useState(() => createMcpController(mcpRoutes.current!)); const [mcpModel, setMcpModel] = useState(() => mcpController.model); @@ -909,20 +910,20 @@ const Workbench = () => { setMcpController(replacement); setMcpModel(replacement.model); }, - foreground: foreground.current!, + foreground: foregroundClient, }); } if (mcpAppClient.current === undefined) { - mcpAppClient.current = new McpAppClient({ foreground: foreground.current!, projectClient: client.current }); + mcpAppClient.current = new McpAppClient({ foreground: foregroundClient, projectClient: client.current }); } if (mcpControllerRef.current !== mcpController) mcpControllerRef.current = mcpController; - if (runtimeClient.current === undefined) runtimeClient.current = new RuntimeClient(foreground.current); - if (artifactClient.current === undefined) artifactClient.current = new ArtifactClient({ foreground: foreground.current! }); - if (comparisonClient.current === undefined) comparisonClient.current = new ComparisonClient({ foreground: foreground.current! }); - if (evalClient.current === undefined) evalClient.current = new EvalClient({ foreground: foreground.current! }); - if (hookClient.current === undefined) hookClient.current = new HookClient({ foreground: foreground.current! }); - if (logClient.current === undefined) logClient.current = new LogClient({ foreground: foreground.current! }); - if (playgroundClient.current === undefined) playgroundClient.current = new PlaygroundClient({ foreground: foreground.current! }); + if (runtimeClient.current === undefined) runtimeClient.current = new RuntimeClient(foregroundClient); + if (artifactClient.current === undefined) artifactClient.current = new ArtifactClient({ foreground: foregroundClient }); + if (comparisonClient.current === undefined) comparisonClient.current = new ComparisonClient({ foreground: foregroundClient }); + if (evalClient.current === undefined) evalClient.current = new EvalClient({ foreground: foregroundClient }); + if (hookClient.current === undefined) hookClient.current = new HookClient({ foreground: foregroundClient }); + if (logClient.current === undefined) logClient.current = new LogClient({ foreground: foregroundClient }); + if (playgroundClient.current === undefined) playgroundClient.current = new PlaygroundClient({ foreground: foregroundClient }); const runtimeAvailable = runtimeCapability === 'available'; const buildId = status === undefined ? undefined : activeEpochId(status);