Skip to content

Commit 0cd08d5

Browse files
qq9340100claude
andauthored
refactor(runtime)!: retire the exported HttpServer delegating wrapper (#5122) (#6141)
`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 Claude-Session: https://claude.ai/code/session_01Wbxm29qPKnLf44AbSxizqW Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7f62706 commit 0cd08d5

4 files changed

Lines changed: 165 additions & 143 deletions

File tree

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
---
2+
"@objectstack/runtime": major
3+
---
4+
5+
refactor(runtime)!: retire the exported `HttpServer` delegating wrapper — it declared `implements IHttpServer` and forwarded none of the contract's optional members (#5122)
6+
7+
**BREAKING.** `@objectstack/runtime` no longer exports `HttpServer`, and
8+
`packages/runtime/src/http-server.ts` is deleted.
9+
10+
## What it was, and why it could not stay
11+
12+
The class took an `IHttpServer` in its constructor and forwarded that server's
13+
**required** members — `get` / `post` / `put` / `delete` / `patch` / `use` /
14+
`listen` / `close` — while declaring `implements IHttpServer`. It forwarded not
15+
one of the contract's **optional** members:
16+
17+
| Optional member | What wrapping it cost |
18+
| --- | --- |
19+
| `getPort?()` | the real bound port after `listen(0)`; harnesses and `@objectstack/http-conformance` address the server through it |
20+
| `getRawApp?()` | the framework-native escape hatch four consumers feature-detect (cloud-connection ×2, metadata's HMR routes, cloud's serverless node server) |
21+
| `setFallbackHandler?()` | since #5111, the **only** entry path there is for declarative `apis:` endpoints |
22+
23+
`packages/spec/src/contracts/http-server.ts` tells consumers to feature-detect
24+
those members with `typeof server.X === 'function'` and to degrade when they are
25+
absent. Wrapping a capable adapter therefore made every probe answer **false**
26+
and the capability disappear — with the adapter underneath providing it the
27+
whole time. Write this row down before reaching for a wrapper of the same shape:
28+
**a host that wrapped `HonoHttpServer` and registered the wrapper as
29+
`http.server` would answer 404 to every endpoint its metadata declared**,
30+
because the seam those endpoints mount through was never forwarded. The dispatcher's
31+
own #5409 declaration — the seam's absence announced at `warn`, welded by
32+
`packages/runtime/src/dispatcher-plugin.fallback-absence-warn.test.ts` — remains
33+
the runtime-side backstop and still fires here, but it can only name the missing
34+
seam; it cannot name the wrapper that swallowed it.
35+
36+
## Migration
37+
38+
**Register an `IHttpServer` adapter INSTANCE, don't wrap one.** Every real host
39+
in this repository already does; `new HttpServer(` had zero occurrences in the
40+
repository, examples included, which is why this retirement carries no rollback
41+
risk and why it is cheaper to take now than later.
42+
43+
| Wrote | Write instead |
44+
| --- | --- |
45+
| `new HttpServer(new HonoHttpServer(port))` registered as `http.server` | register the `HonoHttpServer` (or your own adapter) directly — `HonoServerPlugin` already does |
46+
| 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 |
47+
| `import { HttpServer } from '@objectstack/runtime'` | remove it — `tsc` reports this one, the symbol is simply gone |
48+
49+
Unlike a method quietly dropped from a class, this break is visible to the
50+
compiler: the export does not exist, so nothing type-checks past it. Its absence
51+
from the barrel is additionally pinned at runtime by
52+
`packages/runtime/src/http-server-retirement.test.ts`.
53+
54+
## Why retirement rather than conditional forwarding
55+
56+
Growing a forwarding surface nobody composes would have to be maintained forever
57+
and re-audited every time `IHttpServer` gains an optional member — it gained one
58+
as recently as #5080. The 2026-08-06 maintainer ruling took the #4939
59+
(`ApiRegistry`) precedent instead — retiring a part that was never assembled
60+
beats repairing it — under ADR-0049's remove side.
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#5122] The `HttpServer` delegating wrapper is RETIRED from `@objectstack/runtime`.
5+
*
6+
* ## What was wrong
7+
*
8+
* `packages/runtime/src/http-server.ts` declared `class HttpServer implements
9+
* IHttpServer` and forwarded a constructor-injected server's REQUIRED members
10+
* (`get`/`post`/`put`/`delete`/`patch`/`use`/`listen`/`close`) — and only
11+
* those. Every OPTIONAL member of the contract was dropped on the floor:
12+
*
13+
* - `getPort?()` — how a harness addresses a `listen(0)` ephemeral port;
14+
* - `getRawApp?()` — the framework-native escape hatch four consumers
15+
* feature-detect (cloud-connection ×2, metadata HMR routes, cloud
16+
* serverless);
17+
* - `setFallbackHandler?()` — since the #5040 E7 publish flip landed (#5111),
18+
* the SINGLE seam declarative `apis:` endpoints enter through.
19+
*
20+
* `packages/spec/src/contracts/http-server.ts` tells consumers to probe these
21+
* with `typeof server.X === 'function'`, so wrapping a capable adapter made
22+
* every probe read **false** and the capability vanish — with the underlying
23+
* adapter providing it all along. For `setFallbackHandler` that shape is at
24+
* its worst: a host that wrapped `HonoHttpServer` and registered the wrapper
25+
* as `http.server` would 404 every endpoint its metadata declared, and the
26+
* runtime-side warn (#5409, `dispatcher-plugin.fallback-absence-warn.test.ts`)
27+
* would name the seam's absence without being able to name the wrapper.
28+
*
29+
* ## Why retirement rather than conditional forwarding
30+
*
31+
* `new HttpServer(` had **zero** occurrences in this repository, examples
32+
* included; the class was reachable only as a barrel export. Real hosts
33+
* register an `IHttpServer` adapter INSTANCE as `http.server` and always did.
34+
* The 2026-08-06 maintainer ruling took the #4939 (`ApiRegistry`) precedent —
35+
* retiring a part that was never assembled beats repairing it — over growing a
36+
* forwarding surface nobody composes; ADR-0049's remove side is the process.
37+
*
38+
* ## Why these assertions are runtime probes, not type-level ones
39+
*
40+
* A removed export cannot be imported by name — that would not compile, so the
41+
* pin has to interrogate the namespace object instead. And a compile-time pin
42+
* would be inert here anyway: `packages/runtime/tsconfig.json` excludes
43+
* `**\/*.test.ts` from `tsc --noEmit`, and vitest never type-checks (#4311),
44+
* which is the #4642 trap the sibling pin in
45+
* `packages/spec/src/api/registry-retirement.test.ts` records. Hence runtime
46+
* probes, each with an anti-vacuity guard so a broken import or a wrong path
47+
* cannot turn "absent" into a free pass.
48+
*/
49+
50+
import { existsSync } from 'node:fs';
51+
import path from 'node:path';
52+
import { fileURLToPath } from 'node:url';
53+
54+
import { describe, it, expect } from 'vitest';
55+
56+
// Statically, not `await import()` inside a case: this barrel pulls the whole
57+
// runtime (sandbox, rest, security, observability) and takes several seconds to
58+
// evaluate cold — long enough to blow vitest's 5s per-test timeout, which is a
59+
// flake, not a finding. A namespace object answers `in` and `Object.keys` just
60+
// as well, and module evaluation happens outside any test's clock.
61+
import * as runtime from './index.js';
62+
63+
const SRC_DIR = path.dirname(fileURLToPath(import.meta.url));
64+
65+
describe('[#5122] `HttpServer` wrapper retired from @objectstack/runtime', () => {
66+
it('the barrel exports no `HttpServer`', () => {
67+
// Anti-vacuity FIRST: the namespace we are about to probe must be real and
68+
// non-trivial, or the `toBe(false)` below passes for the wrong reason.
69+
const names = Object.keys(runtime);
70+
expect(names.length, 'the runtime barrel must export a non-trivial surface').toBeGreaterThan(40);
71+
expect(names).toContain('Runtime');
72+
73+
expect('HttpServer' in runtime, '@objectstack/runtime must not export HttpServer').toBe(false);
74+
});
75+
76+
it('keeps the HTTP exports that were its neighbours — the deletion took nothing with it', () => {
77+
for (const kept of ['HttpDispatcher', 'DomainHandlerRegistry', 'MiddlewareManager']) {
78+
expect(kept in runtime, `${kept} must survive the retirement`).toBe(true);
79+
}
80+
});
81+
82+
it('the module file is gone, so it cannot come back as an unexported private wrapper', () => {
83+
// Anti-vacuity: a wrong base directory would make every `existsSync` false.
84+
expect(
85+
existsSync(path.join(SRC_DIR, 'http-dispatcher.ts')),
86+
'probe path must point at the runtime source directory',
87+
).toBe(true);
88+
89+
expect(
90+
existsSync(path.join(SRC_DIR, 'http-server.ts')),
91+
'packages/runtime/src/http-server.ts is retired (#5122) — a host composes the ' +
92+
'framework by registering an IHttpServer ADAPTER INSTANCE as `http.server`, ' +
93+
'not by wrapping one in a same-shaped delegator that drops the optional members',
94+
).toBe(false);
95+
});
96+
});

packages/runtime/src/http-server.ts

Lines changed: 0 additions & 142 deletions
This file was deleted.

packages/runtime/src/index.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,15 @@ export { createSystemEnvironmentPlugin, SYSTEM_ENVIRONMENT_ID } from './system-e
4545
export type { SystemEnvironmentPluginConfig } from './system-environment-plugin.js';
4646

4747
// Export HTTP Server Components
48-
export { HttpServer } from './http-server.js';
48+
// NOTE: the `HttpServer` delegating wrapper (`./http-server.ts`) is RETIRED
49+
// (#5122, #4939 precedent). It declared `implements IHttpServer` but forwarded
50+
// only the REQUIRED members, so every optional one — `getPort`, `getRawApp`
51+
// and above all `setFallbackHandler`, the single seam declarative `apis:`
52+
// endpoints enter through since #5111 — read as absent to the `typeof x ===
53+
// 'function'` probe the contract tells consumers to use. Do not reintroduce a
54+
// same-shaped wrapper: a host composes the framework by registering an
55+
// `IHttpServer` ADAPTER INSTANCE as `http.server`, which is what every real
56+
// host already does. Absence is held by `http-server-retirement.test.ts`.
4957
export { HttpDispatcher } from './http-dispatcher.js';
5058
export type { HttpProtocolContext, HttpDispatcherResult } from './http-dispatcher.js';
5159
// ADR-0006 generic kernel-resolution seam (retained framework contract; the

0 commit comments

Comments
 (0)