From 3dec4e3472b66af067a9b58631a1ff4df19e41c2 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 24 Aug 2026 08:09:33 -0700 Subject: [PATCH] refactor(gateway): unify maintenance lifecycle ownership (#128766) --- config/assertion-safety-baseline.txt | 2 +- src/gateway/server-close.test.ts | 14 +++++++++----- src/gateway/server-close.ts | 19 ++++++++----------- src/gateway/server-lifecycle.ts | 21 +++------------------ src/gateway/server-runtime-handles.test.ts | 5 ----- src/gateway/server-runtime-handles.ts | 22 +++------------------- src/gateway/server-runtime-services.ts | 2 +- src/gateway/server-startup-finish.ts | 7 +------ 8 files changed, 26 insertions(+), 66 deletions(-) diff --git a/config/assertion-safety-baseline.txt b/config/assertion-safety-baseline.txt index 8bdc6b31ce32..1e123d4377ea 100644 --- a/config/assertion-safety-baseline.txt +++ b/config/assertion-safety-baseline.txt @@ -3052,7 +3052,7 @@ src/gateway/server-plugins.ts 2 src/gateway/server-reload-restart.ts 2 src/gateway/server-reload-utils.ts 1 src/gateway/server-restart-sentinel-agent-delivery.ts 2 -src/gateway/server-runtime-handles.ts 11 +src/gateway/server-runtime-handles.ts 9 src/gateway/server-runtime-state-prepare.ts 2 src/gateway/server-session-events.ts 1 src/gateway/server-startup-bootstrap.ts 1 diff --git a/src/gateway/server-close.test.ts b/src/gateway/server-close.test.ts index 32bda0871dee..3ba59e5fe394 100644 --- a/src/gateway/server-close.test.ts +++ b/src/gateway/server-close.test.ts @@ -164,12 +164,16 @@ function createGatewayCloseTestDeps( stopTaskRegistryMaintenance: null, nodePresenceTimers: new Map(), broadcast: vi.fn(), - tickInterval: setInterval(() => undefined, 60_000), - healthInterval: setInterval(() => undefined, 60_000), - dedupeCleanup: setInterval(() => undefined, 60_000), + maintenance: { + tickInterval: setInterval(() => undefined, 60_000), + healthInterval: setInterval(() => undefined, 60_000), + dedupeCleanup: setInterval(() => undefined, 60_000), + startMediaCleanup: vi.fn(), + stopMediaCleanup: vi.fn(async () => "drained" as const), + worktreeCleanup: setInterval(() => undefined, 60_000), + skillCuratorCleanup: vi.fn(), + }, stopMediaCleanup: vi.fn(async () => "drained" as const), - worktreeCleanup: null, - skillCuratorCleanup: vi.fn(), agentUnsub: null, taskUnsub: null, heartbeatUnsub: null, diff --git a/src/gateway/server-close.ts b/src/gateway/server-close.ts index 6bb971054994..52331e075418 100644 --- a/src/gateway/server-close.ts +++ b/src/gateway/server-close.ts @@ -38,6 +38,7 @@ import { } from "./server-chat-state.js"; import type { MediaCleanupStopResult } from "./server-media-cleanup-lifecycle.js"; import { clearSessionTypingState } from "./server-methods/session-typing-state.js"; +import type { GatewayMaintenanceHandles } from "./server-runtime-services.js"; const shutdownLog = createSubsystemLogger("gateway/shutdown"); const GATEWAY_SHUTDOWN_HOOK_TIMEOUT_MS = 5_000; @@ -728,12 +729,8 @@ export function createGatewayCloseHandler( updateCheckStop?: (() => void) | null; stopTaskRegistryMaintenance?: (() => Promise | void) | null; nodePresenceTimers: Map>; - tickInterval: ReturnType; - healthInterval: ReturnType; - dedupeCleanup: ReturnType; + maintenance: GatewayMaintenanceHandles | null; stopMediaCleanup: () => Promise; - worktreeCleanup: ReturnType | null; - skillCuratorCleanup: () => void; agentUnsub: (() => Promise | void) | null; heartbeatUnsub: (() => void) | null; transcriptUnsub: (() => void) | null; @@ -976,13 +973,13 @@ export function createGatewayCloseHandler( reason, restartExpectedMs, }); - clearInterval(params.tickInterval); - clearInterval(params.healthInterval); - clearInterval(params.dedupeCleanup); - if (params.worktreeCleanup) { - clearInterval(params.worktreeCleanup); + if (params.maintenance) { + clearInterval(params.maintenance.tickInterval); + clearInterval(params.maintenance.healthInterval); + clearInterval(params.maintenance.dedupeCleanup); + clearInterval(params.maintenance.worktreeCleanup); + params.maintenance.skillCuratorCleanup(); } - params.skillCuratorCleanup(); if (params.agentUnsub) { await shutdownStep("agent-unsub", () => params.agentUnsub!(), warnings); } diff --git a/src/gateway/server-lifecycle.ts b/src/gateway/server-lifecycle.ts index 8a5243a00f68..b5803745d21b 100644 --- a/src/gateway/server-lifecycle.ts +++ b/src/gateway/server-lifecycle.ts @@ -324,20 +324,9 @@ export async function prepareGatewayLifecycle(params: { addGatewayLifetimeSidecar: (sidecar: (typeof runtimeState.gatewayLifetimeSidecars)[number]) => { runtimeState.gatewayLifetimeSidecars.push(sidecar); }, - setMaintenanceHandles: (handles: { - tickInterval: typeof runtimeState.tickInterval; - healthInterval: typeof runtimeState.healthInterval; - dedupeCleanup: typeof runtimeState.dedupeCleanup; - stopMediaCleanup: typeof runtimeState.stopMediaCleanup; - worktreeCleanup: typeof runtimeState.worktreeCleanup; - skillCuratorCleanup: typeof runtimeState.skillCuratorCleanup; - }) => { - runtimeState.tickInterval = handles.tickInterval; - runtimeState.healthInterval = handles.healthInterval; - runtimeState.dedupeCleanup = handles.dedupeCleanup; + setMaintenanceHandles: (handles: NonNullable) => { + runtimeState.maintenance = handles; runtimeState.stopMediaCleanup = handles.stopMediaCleanup; - runtimeState.worktreeCleanup = handles.worktreeCleanup; - runtimeState.skillCuratorCleanup = handles.skillCuratorCleanup; }, }; runtimeState.controlUiSessionPullRequests = createControlUiSessionPullRequestSubscriptions({ @@ -524,12 +513,8 @@ export async function prepareGatewayLifecycle(params: { stopTaskRegistryMaintenance: shutdownRuntime.stopTaskRegistryMaintenance, nodePresenceTimers, broadcast, - tickInterval: runtimeState.tickInterval, - healthInterval: runtimeState.healthInterval, - dedupeCleanup: runtimeState.dedupeCleanup, + maintenance: runtimeState.maintenance, stopMediaCleanup: stopMediaCleanupForClose, - worktreeCleanup: runtimeState.worktreeCleanup, - skillCuratorCleanup: runtimeState.skillCuratorCleanup, agentUnsub: runtimeState.agentUnsub, heartbeatUnsub: runtimeState.heartbeatUnsub, transcriptUnsub: runtimeState.transcriptUnsub, diff --git a/src/gateway/server-runtime-handles.test.ts b/src/gateway/server-runtime-handles.test.ts index 8ad828977eb8..f42559786f59 100644 --- a/src/gateway/server-runtime-handles.test.ts +++ b/src/gateway/server-runtime-handles.test.ts @@ -30,10 +30,5 @@ describe("createGatewayServerMutableState", () => { await vi.advanceTimersByTimeAsync(0); const restartedState = createGatewayServerMutableState(); await expect(restartedState.stopMediaCleanup()).resolves.toBe("drained"); - for (const runtimeState of [state, restartedState]) { - clearInterval(runtimeState.tickInterval); - clearInterval(runtimeState.healthInterval); - clearInterval(runtimeState.dedupeCleanup); - } }); }); diff --git a/src/gateway/server-runtime-handles.ts b/src/gateway/server-runtime-handles.ts index 2ea5d95754ed..21303658a0c2 100644 --- a/src/gateway/server-runtime-handles.ts +++ b/src/gateway/server-runtime-handles.ts @@ -9,6 +9,7 @@ import { waitForMediaCleanupDrains, } from "./server-media-cleanup-lifecycle.js"; import { createNoopHeartbeatRunner } from "./server-runtime-service-shared.js"; +import type { GatewayMaintenanceHandles } from "./server-runtime-services.js"; import type { GatewayPostReadySidecarHandle } from "./server-startup-post-attach.js"; // Mutable server handles track timers, sidecars, subscriptions, and service @@ -25,12 +26,8 @@ export type GatewayConfigReloaderHandle = { /** Mutable handles owned by a running gateway server process. */ export type GatewayServerMutableState = { bonjourStop: (() => Promise) | null; - tickInterval: ReturnType; - healthInterval: ReturnType; - dedupeCleanup: ReturnType; + maintenance: GatewayMaintenanceHandles | null; stopMediaCleanup: () => Promise; - worktreeCleanup: ReturnType | null; - skillCuratorCleanup: () => void; heartbeatRunner: HeartbeatRunner; stopOutboundDeliveryRecovery: () => Promise; stopGatewayUpdateCheck: () => void; @@ -41,7 +38,6 @@ export type GatewayServerMutableState = { skillsRefreshDelayMs: number; skillsChangeUnsub: () => Promise; channelHealthMonitor: ChannelHealthMonitor | null; - mcpServer: { port: number; close: () => Promise } | undefined; configReloader: GatewayConfigReloaderHandle; agentUnsub: (() => Promise | void) | null; heartbeatUnsub: (() => void) | null; @@ -52,21 +48,10 @@ export type GatewayServerMutableState = { /** Creates gateway mutable state with inert handles that are safe to stop before startup finishes. */ export function createGatewayServerMutableState(): GatewayServerMutableState { - const noopInterval = () => { - // Dummy unref'd timers give shutdown code a concrete handle to clear even - // when startup exits before real maintenance intervals are installed. - const timer = setInterval(() => {}, 1 << 30); - timer.unref?.(); - return timer; - }; return { bonjourStop: null as (() => Promise) | null, - tickInterval: noopInterval(), - healthInterval: noopInterval(), - dedupeCleanup: noopInterval(), + maintenance: null, stopMediaCleanup: () => waitForMediaCleanupDrains({ timeoutMs: MEDIA_CLEANUP_STOP_TIMEOUT_MS }), - worktreeCleanup: null as ReturnType | null, - skillCuratorCleanup: () => {}, heartbeatRunner: createNoopHeartbeatRunner(), stopOutboundDeliveryRecovery: async () => {}, stopGatewayUpdateCheck: () => {}, @@ -77,7 +62,6 @@ export function createGatewayServerMutableState(): GatewayServerMutableState { skillsRefreshDelayMs: 30_000, skillsChangeUnsub: async () => {}, channelHealthMonitor: null as ChannelHealthMonitor | null, - mcpServer: undefined as { port: number; close: () => Promise } | undefined, configReloader: { stop: async () => {}, notifyPluginMetadataChanged: () => {}, diff --git a/src/gateway/server-runtime-services.ts b/src/gateway/server-runtime-services.ts index 8d854c4ad8c1..b659ef6ad379 100644 --- a/src/gateway/server-runtime-services.ts +++ b/src/gateway/server-runtime-services.ts @@ -76,7 +76,7 @@ export function startGatewayCronWithLogging(params: { }).catch((err: unknown) => params.logCron.error(`failed to enter start root: ${String(err)}`)); } -async function clearGatewayMaintenanceHandles( +export async function clearGatewayMaintenanceHandles( maintenance: GatewayMaintenanceHandles | null, ): Promise { if (!maintenance) { diff --git a/src/gateway/server-startup-finish.ts b/src/gateway/server-startup-finish.ts index 544c76384a9f..6bd95ce2653c 100644 --- a/src/gateway/server-startup-finish.ts +++ b/src/gateway/server-startup-finish.ts @@ -448,12 +448,7 @@ export async function finishGatewayStartup(params: { }, applyMaintenance: async (maintenance) => { if (lifecycle.closePreludeStarted) { - clearInterval(maintenance.tickInterval); - clearInterval(maintenance.healthInterval); - clearInterval(maintenance.dedupeCleanup); - await maintenance.stopMediaCleanup(); - clearInterval(maintenance.worktreeCleanup); - maintenance.skillCuratorCleanup(); + await gatewayRuntimeServices.clearGatewayMaintenanceHandles(maintenance); return; } // Publish the stop owner before cleanup can touch SQLite or state paths;