diff --git a/src/commands/serve-http.ts b/src/commands/serve-http.ts index 19f2d74a7..85ac0307f 100644 --- a/src/commands/serve-http.ts +++ b/src/commands/serve-http.ts @@ -12,6 +12,7 @@ import express from 'express'; import type { Request, Response, NextFunction } from 'express'; +import type { Server as HttpServer } from 'http'; import cookieParser from 'cookie-parser'; import cors from 'cors'; import rateLimit from 'express-rate-limit'; @@ -46,6 +47,7 @@ import { type IngestionEvent, } from '../core/ingestion/types.ts'; import { resolveOwnerHolder } from '../core/owner-holder.ts'; +import { registerCleanup } from '../core/process-cleanup.ts'; /** * /health endpoint timeout. 3s rather than 5s: Fly.io's default @@ -55,6 +57,69 @@ import { resolveOwnerHolder } from '../core/owner-holder.ts'; */ export const HEALTH_TIMEOUT_MS = 3000; +type HttpServerLifecycle = Pick; +type SignalSource = Pick; +type CleanupRegistrar = typeof registerCleanup; + +/** + * Keep the HTTP server strongly referenced and make the daemon lifetime + * explicit instead of relying on runtime-specific event-loop behavior for an + * unobserved `app.listen()` return value. The shared abnormal-termination + * cleanup pass closes it before process exit. + */ +export function waitForHttpServerLifecycle( + server: HttpServerLifecycle, + options: { + signals?: SignalSource; + register?: CleanupRegistrar; + } = {}, +): Promise { + const signals = options.signals ?? process; + const register = options.register ?? registerCleanup; + + return new Promise((resolve, reject) => { + let settled = false; + let closePromise: Promise | null = null; + + const closeServer = (): Promise => { + if (closePromise) return closePromise; + closePromise = new Promise((closeResolve, closeReject) => { + if (!server.listening) { + closeResolve(); + return; + } + server.close((error?: Error) => { + if (error) closeReject(error); + else closeResolve(); + }); + }); + return closePromise; + }; + + const deregister = register('http-server', closeServer); + + const finish = (error?: Error) => { + if (settled) return; + settled = true; + server.off('close', onClose); + server.off('error', onError); + signals.off('SIGINT', onSigint); + deregister(); + if (error) reject(error); + else resolve(); + }; + const onClose = () => finish(); + const onError = (error: Error) => finish(error); + const onSigint = () => { + void closeServer().catch(onError); + }; + + server.once('close', onClose); + server.once('error', onError); + signals.once('SIGINT', onSigint); + }); +} + /** * v0.36.1.x #1024: bootstrap token resolution. * @@ -2410,7 +2475,7 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption // --------------------------------------------------------------------------- const clientCount = await sql`SELECT count(*)::int as count FROM oauth_clients`; - app.listen(port, bind, () => { + const httpServer = app.listen(port, bind, () => { console.error(` ╔══════════════════════════════════════════════════════╗ ║ GBrain MCP Server v${VERSION.padEnd(37)}║ @@ -2435,4 +2500,6 @@ ${bootstrapFromEnv : `║ Admin Token (paste into /admin login): ║\n║ ${bootstrapToken.substring(0, 50)} ║\n║ ${bootstrapToken.substring(50).padEnd(50)} ║\n╚══════════════════════════════════════════════════════╝`} `); }); + + await waitForHttpServerLifecycle(httpServer); } diff --git a/test/serve-http-lifecycle.test.ts b/test/serve-http-lifecycle.test.ts new file mode 100644 index 000000000..52c556a03 --- /dev/null +++ b/test/serve-http-lifecycle.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, test } from 'bun:test'; +import { EventEmitter } from 'events'; +import { waitForHttpServerLifecycle } from '../src/commands/serve-http.ts'; + +class FakeHttpServer extends EventEmitter { + listening = true; + closeCalls = 0; + + close(callback?: (error?: Error) => void): this { + this.closeCalls++; + this.listening = false; + queueMicrotask(() => { + callback?.(); + this.emit('close'); + }); + return this; + } +} + +describe('HTTP server lifecycle', () => { + test('waits for shared cleanup to close the server', async () => { + const server = new FakeHttpServer(); + const signals = new EventEmitter(); + let cleanup: (() => Promise) | undefined; + let deregistered = false; + let resolved = false; + + const lifecycle = waitForHttpServerLifecycle(server, { + signals, + register(_name, fn) { + cleanup = fn; + return () => { deregistered = true; }; + }, + }).then(() => { resolved = true; }); + + await Promise.resolve(); + expect(resolved).toBe(false); + expect(cleanup).toBeDefined(); + + await cleanup!(); + await lifecycle; + + expect(server.closeCalls).toBe(1); + expect(deregistered).toBe(true); + expect(signals.listenerCount('SIGINT')).toBe(0); + }); + + test('SIGINT closes the server through the same idempotent path', async () => { + const server = new FakeHttpServer(); + const signals = new EventEmitter(); + + const lifecycle = waitForHttpServerLifecycle(server, { + signals, + register() { + return () => {}; + }, + }); + + signals.emit('SIGINT'); + await lifecycle; + + expect(server.closeCalls).toBe(1); + }); +});