From c77af92654956cc7e0d06420d459ec929b5dec5e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 03:16:58 +0000 Subject: [PATCH] refactor(runtime)!: retire the exported HttpServer delegating wrapper (#5122) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `HttpServer implements IHttpServer` forwarded only the contract's REQUIRED members, so `getPort` / `getRawApp` / `setFallbackHandler` all read as absent to the `typeof x === 'function'` probe the contract prescribes — a wrapped adapter lost every optional capability it actually provided. Since #5111 `setFallbackHandler` is the only entry path for declarative `apis:` endpoints, so wrapping a capable adapter would 404 every declared endpoint silently. `new HttpServer(` had zero occurrences repo-wide (examples included), so the 2026-08-06 maintainer ruling retires the class per the #4939 `ApiRegistry` precedent + ADR-0049's remove side, rather than growing a forwarding surface nobody composes. - delete packages/runtime/src/http-server.ts and its barrel export - pin the absence at runtime (http-server-retirement.test.ts) with anti-vacuity guards; a compile-time pin is inert here (tsconfig excludes **/*.test.ts, #4311/#4642) - changeset: @objectstack/runtime major, with the migration note and the #5111 fact recorded so the wrapper is not reinvented Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Wbxm29qPKnLf44AbSxizqW --- .../runtime-httpserver-wrapper-retired.md | 60 ++++++++ .../src/http-server-retirement.test.ts | 96 ++++++++++++ packages/runtime/src/http-server.ts | 142 ------------------ packages/runtime/src/index.ts | 10 +- 4 files changed, 165 insertions(+), 143 deletions(-) create mode 100644 .changeset/runtime-httpserver-wrapper-retired.md create mode 100644 packages/runtime/src/http-server-retirement.test.ts delete mode 100644 packages/runtime/src/http-server.ts diff --git a/.changeset/runtime-httpserver-wrapper-retired.md b/.changeset/runtime-httpserver-wrapper-retired.md new file mode 100644 index 0000000000..a164e59980 --- /dev/null +++ b/.changeset/runtime-httpserver-wrapper-retired.md @@ -0,0 +1,60 @@ +--- +"@objectstack/runtime": major +--- + +refactor(runtime)!: retire the exported `HttpServer` delegating wrapper — it declared `implements IHttpServer` and forwarded none of the contract's optional members (#5122) + +**BREAKING.** `@objectstack/runtime` no longer exports `HttpServer`, and +`packages/runtime/src/http-server.ts` is deleted. + +## What it was, and why it could not stay + +The class took an `IHttpServer` in its constructor and forwarded that server's +**required** members — `get` / `post` / `put` / `delete` / `patch` / `use` / +`listen` / `close` — while declaring `implements IHttpServer`. It forwarded not +one of the contract's **optional** members: + +| Optional member | What wrapping it cost | +| --- | --- | +| `getPort?()` | the real bound port after `listen(0)`; harnesses and `@objectstack/http-conformance` address the server through it | +| `getRawApp?()` | the framework-native escape hatch four consumers feature-detect (cloud-connection ×2, metadata's HMR routes, cloud's serverless node server) | +| `setFallbackHandler?()` | since #5111, the **only** entry path there is for declarative `apis:` endpoints | + +`packages/spec/src/contracts/http-server.ts` tells consumers to feature-detect +those members with `typeof server.X === 'function'` and to degrade when they are +absent. Wrapping a capable adapter therefore made every probe answer **false** +and the capability disappear — with the adapter underneath providing it the +whole time. Write this row down before reaching for a wrapper of the same shape: +**a host that wrapped `HonoHttpServer` and registered the wrapper as +`http.server` would answer 404 to every endpoint its metadata declared**, +because the seam those endpoints mount through was never forwarded. The dispatcher's +own #5409 declaration — the seam's absence announced at `warn`, welded by +`packages/runtime/src/dispatcher-plugin.fallback-absence-warn.test.ts` — remains +the runtime-side backstop and still fires here, but it can only name the missing +seam; it cannot name the wrapper that swallowed it. + +## Migration + +**Register an `IHttpServer` adapter INSTANCE, don't wrap one.** Every real host +in this repository already does; `new HttpServer(` had zero occurrences in the +repository, examples included, which is why this retirement carries no rollback +risk and why it is cheaper to take now than later. + +| Wrote | Write instead | +| --- | --- | +| `new HttpServer(new HonoHttpServer(port))` registered as `http.server` | register the `HonoHttpServer` (or your own adapter) directly — `HonoServerPlugin` already does | +| a wrapper of your own to add cross-cutting behaviour | forward **every** member you did not deliberately drop, optional ones included, and re-probe with `typeof` after wrapping; a delegator that narrows the contract silently removes capabilities | +| `import { HttpServer } from '@objectstack/runtime'` | remove it — `tsc` reports this one, the symbol is simply gone | + +Unlike a method quietly dropped from a class, this break is visible to the +compiler: the export does not exist, so nothing type-checks past it. Its absence +from the barrel is additionally pinned at runtime by +`packages/runtime/src/http-server-retirement.test.ts`. + +## Why retirement rather than conditional forwarding + +Growing a forwarding surface nobody composes would have to be maintained forever +and re-audited every time `IHttpServer` gains an optional member — it gained one +as recently as #5080. The 2026-08-06 maintainer ruling took the #4939 +(`ApiRegistry`) precedent instead — retiring a part that was never assembled +beats repairing it — under ADR-0049's remove side. diff --git a/packages/runtime/src/http-server-retirement.test.ts b/packages/runtime/src/http-server-retirement.test.ts new file mode 100644 index 0000000000..276530dbe8 --- /dev/null +++ b/packages/runtime/src/http-server-retirement.test.ts @@ -0,0 +1,96 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5122] The `HttpServer` delegating wrapper is RETIRED from `@objectstack/runtime`. + * + * ## What was wrong + * + * `packages/runtime/src/http-server.ts` declared `class HttpServer implements + * IHttpServer` and forwarded a constructor-injected server's REQUIRED members + * (`get`/`post`/`put`/`delete`/`patch`/`use`/`listen`/`close`) — and only + * those. Every OPTIONAL member of the contract was dropped on the floor: + * + * - `getPort?()` — how a harness addresses a `listen(0)` ephemeral port; + * - `getRawApp?()` — the framework-native escape hatch four consumers + * feature-detect (cloud-connection ×2, metadata HMR routes, cloud + * serverless); + * - `setFallbackHandler?()` — since the #5040 E7 publish flip landed (#5111), + * the SINGLE seam declarative `apis:` endpoints enter through. + * + * `packages/spec/src/contracts/http-server.ts` tells consumers to probe these + * with `typeof server.X === 'function'`, so wrapping a capable adapter made + * every probe read **false** and the capability vanish — with the underlying + * adapter providing it all along. For `setFallbackHandler` that shape is at + * its worst: a host that wrapped `HonoHttpServer` and registered the wrapper + * as `http.server` would 404 every endpoint its metadata declared, and the + * runtime-side warn (#5409, `dispatcher-plugin.fallback-absence-warn.test.ts`) + * would name the seam's absence without being able to name the wrapper. + * + * ## Why retirement rather than conditional forwarding + * + * `new HttpServer(` had **zero** occurrences in this repository, examples + * included; the class was reachable only as a barrel export. Real hosts + * register an `IHttpServer` adapter INSTANCE as `http.server` and always did. + * The 2026-08-06 maintainer ruling took the #4939 (`ApiRegistry`) precedent — + * retiring a part that was never assembled beats repairing it — over growing a + * forwarding surface nobody composes; ADR-0049's remove side is the process. + * + * ## Why these assertions are runtime probes, not type-level ones + * + * A removed export cannot be imported by name — that would not compile, so the + * pin has to interrogate the namespace object instead. And a compile-time pin + * would be inert here anyway: `packages/runtime/tsconfig.json` excludes + * `**\/*.test.ts` from `tsc --noEmit`, and vitest never type-checks (#4311), + * which is the #4642 trap the sibling pin in + * `packages/spec/src/api/registry-retirement.test.ts` records. Hence runtime + * probes, each with an anti-vacuity guard so a broken import or a wrong path + * cannot turn "absent" into a free pass. + */ + +import { existsSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { describe, it, expect } from 'vitest'; + +// Statically, not `await import()` inside a case: this barrel pulls the whole +// runtime (sandbox, rest, security, observability) and takes several seconds to +// evaluate cold — long enough to blow vitest's 5s per-test timeout, which is a +// flake, not a finding. A namespace object answers `in` and `Object.keys` just +// as well, and module evaluation happens outside any test's clock. +import * as runtime from './index.js'; + +const SRC_DIR = path.dirname(fileURLToPath(import.meta.url)); + +describe('[#5122] `HttpServer` wrapper retired from @objectstack/runtime', () => { + it('the barrel exports no `HttpServer`', () => { + // Anti-vacuity FIRST: the namespace we are about to probe must be real and + // non-trivial, or the `toBe(false)` below passes for the wrong reason. + const names = Object.keys(runtime); + expect(names.length, 'the runtime barrel must export a non-trivial surface').toBeGreaterThan(40); + expect(names).toContain('Runtime'); + + expect('HttpServer' in runtime, '@objectstack/runtime must not export HttpServer').toBe(false); + }); + + it('keeps the HTTP exports that were its neighbours — the deletion took nothing with it', () => { + for (const kept of ['HttpDispatcher', 'DomainHandlerRegistry', 'MiddlewareManager']) { + expect(kept in runtime, `${kept} must survive the retirement`).toBe(true); + } + }); + + it('the module file is gone, so it cannot come back as an unexported private wrapper', () => { + // Anti-vacuity: a wrong base directory would make every `existsSync` false. + expect( + existsSync(path.join(SRC_DIR, 'http-dispatcher.ts')), + 'probe path must point at the runtime source directory', + ).toBe(true); + + expect( + existsSync(path.join(SRC_DIR, 'http-server.ts')), + 'packages/runtime/src/http-server.ts is retired (#5122) — a host composes the ' + + 'framework by registering an IHttpServer ADAPTER INSTANCE as `http.server`, ' + + 'not by wrapping one in a same-shaped delegator that drops the optional members', + ).toBe(false); + }); +}); diff --git a/packages/runtime/src/http-server.ts b/packages/runtime/src/http-server.ts deleted file mode 100644 index a62107b9af..0000000000 --- a/packages/runtime/src/http-server.ts +++ /dev/null @@ -1,142 +0,0 @@ -// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. - -import { IHttpServer, RouteHandler, Middleware } from '@objectstack/core'; - -/** - * HttpServer - Unified HTTP Server Abstraction - * - * Provides a framework-agnostic HTTP server interface that wraps - * underlying server implementations (Hono, Express, Fastify, etc.) - * - * This class serves as an adapter between the IHttpServer interface - * and concrete server implementations, allowing plugins to register - * routes and middleware without depending on specific frameworks. - * - * Features: - * - Unified route registration API - * - Middleware management with ordering - * - Request/response lifecycle hooks - * - Framework-agnostic abstractions - */ -export class HttpServer implements IHttpServer { - protected server: IHttpServer; - protected routes: Map; - protected middlewares: Middleware[]; - - /** - * Create an HTTP server wrapper - * @param server - The underlying server implementation (Hono, Express, etc.) - */ - constructor(server: IHttpServer) { - this.server = server; - this.routes = new Map(); - this.middlewares = []; - } - - /** - * Register a GET route handler - * @param path - Route path (e.g., '/api/users/:id') - * @param handler - Route handler function - */ - get(path: string, handler: RouteHandler): void { - const key = `GET:${path}`; - this.routes.set(key, handler); - this.server.get(path, handler); - } - - /** - * Register a POST route handler - * @param path - Route path - * @param handler - Route handler function - */ - post(path: string, handler: RouteHandler): void { - const key = `POST:${path}`; - this.routes.set(key, handler); - this.server.post(path, handler); - } - - /** - * Register a PUT route handler - * @param path - Route path - * @param handler - Route handler function - */ - put(path: string, handler: RouteHandler): void { - const key = `PUT:${path}`; - this.routes.set(key, handler); - this.server.put(path, handler); - } - - /** - * Register a DELETE route handler - * @param path - Route path - * @param handler - Route handler function - */ - delete(path: string, handler: RouteHandler): void { - const key = `DELETE:${path}`; - this.routes.set(key, handler); - this.server.delete(path, handler); - } - - /** - * Register a PATCH route handler - * @param path - Route path - * @param handler - Route handler function - */ - patch(path: string, handler: RouteHandler): void { - const key = `PATCH:${path}`; - this.routes.set(key, handler); - this.server.patch(path, handler); - } - - /** - * Register middleware - * @param path - Optional path to apply middleware to (if omitted, applies globally) - * @param handler - Middleware function - */ - use(path: string | Middleware, handler?: Middleware): void { - if (typeof path === 'function') { - // Global middleware - this.middlewares.push(path); - this.server.use(path); - } else if (handler) { - // Path-specific middleware - this.middlewares.push(handler); - this.server.use(path, handler); - } - } - - /** - * Start the HTTP server - * @param port - Port number to listen on - * @returns Promise that resolves when server is ready - */ - async listen(port: number): Promise { - await this.server.listen(port); - } - - /** - * Stop the HTTP server - * @returns Promise that resolves when server is stopped - */ - async close(): Promise { - if (this.server.close) { - await this.server.close(); - } - } - - /** - * Get registered routes - * @returns Map of route keys to handlers - */ - getRoutes(): Map { - return new Map(this.routes); - } - - /** - * Get registered middlewares - * @returns Array of middleware functions - */ - getMiddlewares(): Middleware[] { - return [...this.middlewares]; - } -} diff --git a/packages/runtime/src/index.ts b/packages/runtime/src/index.ts index b69c8a249e..8b2b981365 100644 --- a/packages/runtime/src/index.ts +++ b/packages/runtime/src/index.ts @@ -45,7 +45,15 @@ export { createSystemEnvironmentPlugin, SYSTEM_ENVIRONMENT_ID } from './system-e export type { SystemEnvironmentPluginConfig } from './system-environment-plugin.js'; // Export HTTP Server Components -export { HttpServer } from './http-server.js'; +// NOTE: the `HttpServer` delegating wrapper (`./http-server.ts`) is RETIRED +// (#5122, #4939 precedent). It declared `implements IHttpServer` but forwarded +// only the REQUIRED members, so every optional one — `getPort`, `getRawApp` +// and above all `setFallbackHandler`, the single seam declarative `apis:` +// endpoints enter through since #5111 — read as absent to the `typeof x === +// 'function'` probe the contract tells consumers to use. Do not reintroduce a +// same-shaped wrapper: a host composes the framework by registering an +// `IHttpServer` ADAPTER INSTANCE as `http.server`, which is what every real +// host already does. Absence is held by `http-server-retirement.test.ts`. export { HttpDispatcher } from './http-dispatcher.js'; export type { HttpProtocolContext, HttpDispatcherResult } from './http-dispatcher.js'; // ADR-0006 generic kernel-resolution seam (retained framework contract; the