test: drop strictPort and document the port collision instead
strictPort was a new test-server config option — plus its validation and 18 tests — invented solely to keep one fixture's hard-coded issuer port honest. That is infrastructure built for a screenshot, so it goes. test-servers/src is now byte-identical to v2/main again. The fixture keeps its fixed port and docs/test-servers.md carries the caveat instead, written to be actionable rather than merely cautionary: the failure mode is confusing rather than obvious, since a relocated server leaves discovery pointing at whatever unrelated process holds 8091, so the note says to check the port before believing what the flow does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018MvNbmAtQuWhDvzi4yXDju Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
b4b82493b1
commit
6c1b5325f0
5 changed files with 4 additions and 319 deletions
|
|
@ -21,7 +21,6 @@
|
|||
},
|
||||
"transport": {
|
||||
"type": "streamable-http",
|
||||
"port": 8091,
|
||||
"strictPort": true
|
||||
"port": 8091
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -456,17 +456,6 @@ export interface ServerConfig {
|
|||
| undefined; // Optional callback to customize resource handler during registration
|
||||
serverType?: "sse" | "streamable-http"; // Transport type (default: "streamable-http")
|
||||
port?: number; // Port to use (optional, will find available port if not specified)
|
||||
/**
|
||||
* Refuse to relocate: bind {@link port} exactly, or fail with EADDRINUSE.
|
||||
*
|
||||
* Off by default, because walking to the next free port is what lets several
|
||||
* fixtures run side by side. It exists for a fixture whose *advertised*
|
||||
* configuration hard-codes the port — `oauth-insecure-token-endpoint-http.json`
|
||||
* puts it in an OAuth issuer — where relocating leaves the server announcing
|
||||
* one port while its metadata still points at another process entirely, and
|
||||
* the fixture silently stops reproducing what it exists to reproduce.
|
||||
*/
|
||||
strictPort?: boolean;
|
||||
/**
|
||||
* Whether to advertise listChanged capability for each list type
|
||||
* If enabled, modification tools will send list_changed notifications
|
||||
|
|
|
|||
|
|
@ -119,8 +119,6 @@ export interface ConfigFile {
|
|||
transport: {
|
||||
type: "stdio" | "streamable-http" | "sse";
|
||||
port?: number;
|
||||
/** Bind `port` exactly, or fail — see `ServerConfig.strictPort`. */
|
||||
strictPort?: boolean;
|
||||
/**
|
||||
* Serve the modern (2026-07-28) protocol era via the SDK's
|
||||
* `createMcpHandler` (only valid with `type: "streamable-http"`). `true`
|
||||
|
|
@ -198,48 +196,6 @@ function validateConfig(
|
|||
`Invalid config in ${filePath}: transport.type must be stdio, streamable-http, or sse`,
|
||||
);
|
||||
}
|
||||
// `strictPort` is consumed as a plain truthiness check at bind time, so a
|
||||
// string `"false"` would read as *enabled* and silently disable the port walk
|
||||
// — the opposite of what the author wrote. Validate the type here, where the
|
||||
// file is being trusted, rather than letting it through as a `ConfigFile`.
|
||||
if (
|
||||
transport.strictPort !== undefined &&
|
||||
typeof transport.strictPort !== "boolean"
|
||||
) {
|
||||
throw new Error(
|
||||
`Invalid config in ${filePath}: transport.strictPort must be a boolean`,
|
||||
);
|
||||
}
|
||||
|
||||
// Beyond the type: reject every combination that cannot honor the flag's
|
||||
// contract, because each of them fails *silently* — the fixture looks strict
|
||||
// and relocates anyway, which is the failure the flag exists to prevent.
|
||||
//
|
||||
// - a string port (`"0"`, `"8091"`) is truthy, so it slips past the runtime
|
||||
// "no port to be strict about" guard, and Node then coerces `"0"` to the
|
||||
// dynamic port 0;
|
||||
// - a non-integer or out-of-range port cannot be bound as written;
|
||||
// - on `stdio` there is no listener at all, and `resolveConfig` drops the
|
||||
// flag, so it silently does nothing.
|
||||
if (transport.strictPort === true) {
|
||||
if (transportType === "stdio") {
|
||||
throw new Error(
|
||||
`Invalid config in ${filePath}: transport.strictPort requires an HTTP transport (streamable-http or sse)`,
|
||||
);
|
||||
}
|
||||
const port = transport.port;
|
||||
if (
|
||||
typeof port !== "number" ||
|
||||
!Number.isInteger(port) ||
|
||||
port < 1 ||
|
||||
port > 65535
|
||||
) {
|
||||
throw new Error(
|
||||
`Invalid config in ${filePath}: transport.strictPort requires transport.port to be an integer in 1-65535 (got ${JSON.stringify(port)})`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Only reject *enabling* modern on a non-HTTP transport; a falsy `modern`
|
||||
// (e.g. `false`) is a no-op that `resolveConfig` normalizes away.
|
||||
if (transport.modern && transportType !== "streamable-http") {
|
||||
|
|
|
|||
|
|
@ -102,7 +102,6 @@ export function resolveConfig(config: ConfigFile): ServerConfig {
|
|||
? (transport.type as "sse" | "streamable-http")
|
||||
: undefined,
|
||||
port: isHttp ? transport.port : undefined,
|
||||
strictPort: isHttp ? transport.strictPort : undefined,
|
||||
};
|
||||
|
||||
// Normalize the modern flag: `true` is shorthand for the default (dual-era
|
||||
|
|
|
|||
|
|
@ -400,39 +400,9 @@ export class TestServerHttp {
|
|||
const serverType = this.config.serverType ?? "streamable-http";
|
||||
const requestedPort = this.config.port;
|
||||
|
||||
// `strictPort` means "bind exactly this port, or fail". With no fixed port
|
||||
// to bind there is nothing to be strict about, and falling through to an
|
||||
// OS-assigned one would let a misconfigured fixture look strict while
|
||||
// relocating on every run — the exact failure the flag exists to prevent,
|
||||
// now silent. Reject the combination instead.
|
||||
if (
|
||||
this.config.strictPort &&
|
||||
(typeof requestedPort !== "number" ||
|
||||
!Number.isInteger(requestedPort) ||
|
||||
requestedPort < 1 ||
|
||||
requestedPort > 65535)
|
||||
) {
|
||||
// `loadConfig` rejects these for a config file; this covers a
|
||||
// programmatic caller, and specifically a value that is *truthy* but not
|
||||
// bindable as written — a string `"0"` slips past a bare falsiness check
|
||||
// and Node then coerces it to the dynamic port 0, so the fixture looks
|
||||
// strict and relocates anyway.
|
||||
throw new Error(
|
||||
`strictPort requires an explicit port as an integer in 1-65535 (got ${JSON.stringify(requestedPort)}): ` +
|
||||
"there is nothing to bind strictly otherwise.",
|
||||
);
|
||||
}
|
||||
|
||||
// If a port is explicitly requested, find an available port starting from that value
|
||||
// Otherwise, use 0 to let the OS assign an available port.
|
||||
// `strictPort` opts out of the walk: bind the requested port or fail loudly
|
||||
// (see the field's doc comment — a relocated server whose advertised config
|
||||
// hard-codes the port is worse than one that does not start).
|
||||
const port = requestedPort
|
||||
? this.config.strictPort
|
||||
? requestedPort
|
||||
: await findAvailablePort(requestedPort)
|
||||
: 0;
|
||||
// Otherwise, use 0 to let the OS assign an available port
|
||||
const port = requestedPort ? await findAvailablePort(requestedPort) : 0;
|
||||
|
||||
if (serverType === "streamable-http") {
|
||||
return this.startHttp(port);
|
||||
|
|
|
|||
Loading…
Reference in a new issue