|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * #12543 — when `os serve` binds a port other than the one it was asked for, it |
| 5 | + * SAYS SO, naming both numbers on one line. |
| 6 | + * |
| 7 | + * ## The defect is the silence, not the behaviour |
| 8 | + * |
| 9 | + * Auto-shifting to the next free port in development is correct and deliberate: |
| 10 | + * several example apps have to run side by side, and #11113 owns the production |
| 11 | + * half, where a busy port is a loud refusal instead. ⛔ Nothing in this file |
| 12 | + * argues for changing either half, and a change to what `os serve` *does* would |
| 13 | + * be answering a different card. What was missing is that the shift happened |
| 14 | + * with nothing marking it as one: the ready banner prints the port that was |
| 15 | + * BOUND, and no line anywhere said it was not the port that was ASKED FOR. |
| 16 | + * |
| 17 | + * ## Why the producer, when the consumers already cope |
| 18 | + * |
| 19 | + * The consumer side of this family is fully landed — a bind probe (#12441), the |
| 20 | + * shared `runServe()` read-back (#12525), three spawners taught to check |
| 21 | + * locally (#12526), and a security probe (#12548). Every one of them |
| 22 | + * re-derives, by hand and by parsing the banner, a fact `serve.ts` holds for |
| 23 | + * free at the moment it shifts: the requested port and the bound port, in one |
| 24 | + * scope. And the banner is a lossy place to re-derive it from — `runServe()`'s |
| 25 | + * read-back carries an `unreadable` state precisely because the `API:` row |
| 26 | + * shows an `OS_AUTH_URL` / `BETTER_AUTH_URL` / `OS_BASE_URL` origin when one is |
| 27 | + * set, which is not what the process bound at all. The producer has no such |
| 28 | + * gap. This file pins it finally saying so. |
| 29 | + * |
| 30 | + * ⚠️ The cost of the silence is measured rather than supposed: a harness asks |
| 31 | + * for a port, silently gets another, and then talks to whatever holds the one |
| 32 | + * it asked for. The positive case below reproduces exactly that — with a real |
| 33 | + * HTTP neighbour, and it asserts the stranger answering as well as the notice. |
| 34 | + * |
| 35 | + * ## CHANNEL — the sharpest constraint here, and not a free choice |
| 36 | + * |
| 37 | + * `stdout` is the JSON-RPC channel whenever the stdio MCP transport is mounted |
| 38 | + * (#7915): one non-frame line there reaches a conforming client as a transport |
| 39 | + * error, which is what `serve-stdio-stdout-purity.e2e.test.ts` exists to pin. |
| 40 | + * So the notice goes to **stderr**, through `serve.ts`'s own `printDiagnostic` |
| 41 | + * — the same helper, stream and boot position as the production-mode refusal |
| 42 | + * that is this notice's counterpart under the other half of the same policy. |
| 43 | + * |
| 44 | + * This file proves the half it can reach on its own: under a real drift the |
| 45 | + * notice is on stderr and the child's stdout carries nothing at all. The other |
| 46 | + * half is that `serve-stdio-stdout-purity.e2e.test.ts` still passes unchanged, |
| 47 | + * which is a different boot shape and stays in its own file. |
| 48 | + * |
| 49 | + * ## Why this file spawns instead of calling `runServe()` |
| 50 | + * |
| 51 | + * `runServe()` REJECTS a drifted boot — that is #12525's read-back doing its |
| 52 | + * job. Routing the positive case through it would make the very condition under |
| 53 | + * test unreachable. So this file owns its spawn, while still taking the |
| 54 | + * entrypoint, the child environment and the free-port draw from the shared |
| 55 | + * helper rather than re-deriving them. |
| 56 | + * |
| 57 | + * ⚠️ The one instrument it does own is the HTTP neighbour. `holdPort()` in the |
| 58 | + * shared helper binds a bare TCP socket, which is enough to make a port |
| 59 | + * unbindable but cannot ANSWER — and "something else answered where you were |
| 60 | + * pointed" is the specific harm this card was filed about. The surface ruling |
| 61 | + * on this card keeps `helpers/serve-process.ts` out of the diff, so the |
| 62 | + * neighbour lives here, named as a deliberate second instrument rather than a |
| 63 | + * blind duplicate of the draw. |
| 64 | + * |
| 65 | + * ## The negative half is not optional |
| 66 | + * |
| 67 | + * ⭐ A drift notice that appears when there is no drift is worse than silence: |
| 68 | + * it trains readers to skip the line, and then the one boot that really did |
| 69 | + * shift reads like every other. The SAME regex is asserted present in the |
| 70 | + * drifted boot and absent in the clean one, so neither case can pass by the |
| 71 | + * regex having quietly stopped matching anything at all. |
| 72 | + */ |
| 73 | + |
| 74 | +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; |
| 75 | +import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process'; |
| 76 | +import { createServer, type Server } from 'node:http'; |
| 77 | +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; |
| 78 | +import { tmpdir } from 'node:os'; |
| 79 | +import { join } from 'node:path'; |
| 80 | + |
| 81 | +import { CLI, TSX, E2E_SECRET_KEY, childEnv, randomPort } from './helpers/serve-process.js'; |
| 82 | + |
| 83 | +/** |
| 84 | + * The notice, as ONE regex shared by both cases. |
| 85 | + * |
| 86 | + * Deliberately loose about decoration and exact about the two numbers: what is |
| 87 | + * pinned is that a reader gets the requested port AND the bound port out of a |
| 88 | + * single line, without holding a second one beside it to compare against. |
| 89 | + */ |
| 90 | +const DRIFT_NOTICE = /Port (\d+) is in use — serving on (\d+) instead\./; |
| 91 | + |
| 92 | +/** The banner's last line — the marker that the WHOLE banner is in the buffer. */ |
| 93 | +const BANNER_TAIL = /Press Ctrl\+C to stop/; |
| 94 | + |
| 95 | +/** The platform with no application — the cheapest fixture that still boots. */ |
| 96 | +const BARE_CONFIG = 'export default {};\n'; |
| 97 | + |
| 98 | +let dir: string; |
| 99 | +const children: ChildProcessWithoutNullStreams[] = []; |
| 100 | + |
| 101 | +beforeAll(() => { |
| 102 | + dir = mkdtempSync(join(tmpdir(), 'os-port-drift-notice-')); |
| 103 | + writeFileSync(join(dir, 'objectstack.config.ts'), BARE_CONFIG, 'utf8'); |
| 104 | +}); |
| 105 | + |
| 106 | +afterAll(async () => { |
| 107 | + for (const child of children) await stop(child); |
| 108 | + if (dir) rmSync(dir, { recursive: true, force: true }); |
| 109 | +}, 60_000); |
| 110 | + |
| 111 | +/** |
| 112 | + * Stop a spawned child and WAIT for it to be gone — SIGTERM first, SIGKILL only |
| 113 | + * as a fallback, which is the shape every other spawner in this directory uses. |
| 114 | + * |
| 115 | + * ⚠️ ⛔ Not a bare `child.kill('SIGKILL')`. The child here is the `tsx` shim, |
| 116 | + * and the `os serve` process is its own child: SIGKILL cannot be forwarded, so |
| 117 | + * killing the shim outright leaves the server running, re-parented to init and |
| 118 | + * still holding this process's stdio pipes — measured while writing this file, |
| 119 | + * where it kept the runner alive after the assertions had all passed. SIGTERM |
| 120 | + * reaches the server through the shim; the 10s SIGKILL is the fallback for a |
| 121 | + * child that ignores it. |
| 122 | + */ |
| 123 | +async function stop(child: ChildProcessWithoutNullStreams): Promise<void> { |
| 124 | + if (child.exitCode !== null || child.signalCode !== null) return; |
| 125 | + await new Promise<void>((done) => { |
| 126 | + const give = setTimeout(() => { |
| 127 | + try { |
| 128 | + child.kill('SIGKILL'); |
| 129 | + } catch { |
| 130 | + /* already gone */ |
| 131 | + } |
| 132 | + done(); |
| 133 | + }, 10_000); |
| 134 | + child.once('exit', () => { |
| 135 | + clearTimeout(give); |
| 136 | + done(); |
| 137 | + }); |
| 138 | + try { |
| 139 | + child.kill('SIGTERM'); |
| 140 | + } catch { |
| 141 | + clearTimeout(give); |
| 142 | + done(); |
| 143 | + } |
| 144 | + }); |
| 145 | +} |
| 146 | + |
| 147 | +/** |
| 148 | + * A real HTTP neighbour on a real port — the card's own instrument, and the |
| 149 | + * reason it must not be simulated. The point is not merely that the port cannot |
| 150 | + * be bound; it is that something ELSE answers there while `os serve` is happily |
| 151 | + * bound somewhere the caller was never told about. |
| 152 | + */ |
| 153 | +function realNeighbour(): Promise<{ port: number; release: () => Promise<void> }> { |
| 154 | + return new Promise((resolveHold, rejectHold) => { |
| 155 | + const server: Server = createServer((_req, res) => { |
| 156 | + res.writeHead(200, { 'content-type': 'application/json' }); |
| 157 | + res.end(JSON.stringify({ iAm: 'A NEIGHBOURING AGENT DEV SERVER, not os serve' })); |
| 158 | + }); |
| 159 | + server.on('error', rejectHold); |
| 160 | + server.listen(0, '0.0.0.0', () => { |
| 161 | + const address = server.address(); |
| 162 | + if (address === null || typeof address === 'string') { |
| 163 | + rejectHold(new Error(`listen(0) produced no numeric address: ${String(address)}`)); |
| 164 | + return; |
| 165 | + } |
| 166 | + resolveHold({ |
| 167 | + port: address.port, |
| 168 | + release: () => new Promise<void>((done) => server.close(() => done())), |
| 169 | + }); |
| 170 | + }); |
| 171 | + }); |
| 172 | +} |
| 173 | + |
| 174 | +interface Booted { |
| 175 | + stdout: string; |
| 176 | + stderr: string; |
| 177 | + child: ChildProcessWithoutNullStreams; |
| 178 | + reachedBanner: boolean; |
| 179 | +} |
| 180 | + |
| 181 | +/** |
| 182 | + * Boot `os serve` on `port` and collect BOTH streams until the banner's last |
| 183 | + * line lands — or the child dies, or the clock runs out. A boot that never got |
| 184 | + * there resolves with `reachedBanner: false` rather than throwing, so the |
| 185 | + * caller's failure message can show what it printed on the way down. |
| 186 | + */ |
| 187 | +function boot(port: number | string, timeoutMs = 180_000): Promise<Booted> { |
| 188 | + return new Promise((resolveBoot) => { |
| 189 | + const child = spawn(TSX, [CLI, 'serve', 'objectstack.config.ts', '--port', String(port)], { |
| 190 | + cwd: dir, |
| 191 | + // The shared child environment, never a bare `...process.env` (#11267). |
| 192 | + // |
| 193 | + // `NODE_ENV` is declared here rather than left to the entrypoint even |
| 194 | + // though `bin/run-dev.js` pins the same value before argv is parsed: the |
| 195 | + // branch under test is `flags.dev || NODE_ENV === 'development'`, so the |
| 196 | + // reason these boots can drift at all belongs AT the spawn where a reader |
| 197 | + // can see it, not two files away. Silence would also inherit the vitest |
| 198 | + // worker's `NODE_ENV=test`, which `childEnv()` deliberately does not |
| 199 | + // strip. |
| 200 | + env: childEnv({ |
| 201 | + NODE_ENV: 'development', |
| 202 | + NO_COLOR: '1', |
| 203 | + OS_DATABASE_URL: ':memory:', |
| 204 | + OS_LOG_LEVEL: '', |
| 205 | + OS_DISABLE_CONSOLE: '1', |
| 206 | + OS_SECRET_KEY: E2E_SECRET_KEY, |
| 207 | + }), |
| 208 | + }); |
| 209 | + children.push(child); |
| 210 | + |
| 211 | + let stdout = ''; |
| 212 | + let stderr = ''; |
| 213 | + let settled = false; |
| 214 | + const done = (reachedBanner: boolean) => { |
| 215 | + if (settled) return; |
| 216 | + settled = true; |
| 217 | + clearTimeout(timer); |
| 218 | + resolveBoot({ stdout, stderr, child, reachedBanner }); |
| 219 | + }; |
| 220 | + |
| 221 | + const timer = setTimeout(() => done(false), timeoutMs); |
| 222 | + // Key on the banner's LAST line so no assertion can read a boot that has |
| 223 | + // printed only half of it. |
| 224 | + const check = () => { |
| 225 | + if (BANNER_TAIL.test(stdout + stderr)) done(true); |
| 226 | + }; |
| 227 | + child.stdout.on('data', (d) => { |
| 228 | + stdout += String(d); |
| 229 | + check(); |
| 230 | + }); |
| 231 | + child.stderr.on('data', (d) => { |
| 232 | + stderr += String(d); |
| 233 | + check(); |
| 234 | + }); |
| 235 | + child.on('exit', () => done(false)); |
| 236 | + }); |
| 237 | +} |
| 238 | + |
| 239 | +/** The port the banner says was actually bound, read out of its `API:` row. */ |
| 240 | +function boundPort(output: string): string | undefined { |
| 241 | + const apiRow = output.match(/^[^\n]*\bAPI:[^\n]*$/m); |
| 242 | + return apiRow ? (apiRow[0].match(/localhost:(\d+)/) ?? [])[1] : undefined; |
| 243 | +} |
| 244 | + |
| 245 | +describe('#12543: a shifted port announces itself, naming both numbers', () => { |
| 246 | + it('POSITIVE — a really-held port produces a notice carrying REQUESTED and BOUND', async () => { |
| 247 | + const neighbour = await realNeighbour(); |
| 248 | + try { |
| 249 | + const booted = await boot(neighbour.port); |
| 250 | + const { stdout, stderr } = booted; |
| 251 | + const seen = `\n--- stdout ---\n${stdout.slice(0, 2000)}\n--- stderr (tail) ---\n${stderr.slice(-3000)}`; |
| 252 | + |
| 253 | + expect(booted.reachedBanner, `the boot never reached its banner${seen}`).toBe(true); |
| 254 | + |
| 255 | + // The drift really happened. Without this, every assertion below could |
| 256 | + // pass on a boot that never shifted at all. |
| 257 | + const bound = boundPort(stderr); |
| 258 | + expect(bound, `no API row in the banner to read a bound port from${seen}`).toBeDefined(); |
| 259 | + expect( |
| 260 | + Number(bound), |
| 261 | + `the child bound the port it was asked for — there is no drift to announce${seen}`, |
| 262 | + ).not.toBe(neighbour.port); |
| 263 | + |
| 264 | + // ── The pin ────────────────────────────────────────────────────── |
| 265 | + const match = stderr.match(DRIFT_NOTICE); |
| 266 | + expect(match, `no drift notice on stderr for a boot that DID drift${seen}`).not.toBeNull(); |
| 267 | + // Both numbers, in that one line. The reader's two questions are "from |
| 268 | + // what" and "to what", and a line answering only the second is the |
| 269 | + // banner — which is what five consumer PRs already had to parse. |
| 270 | + expect(match![1], `the notice does not name the REQUESTED port${seen}`).toBe(String(neighbour.port)); |
| 271 | + expect(match![2], `the notice does not name the BOUND port${seen}`).toBe(String(bound)); |
| 272 | + |
| 273 | + // ── The channel ────────────────────────────────────────────────── |
| 274 | + // stdout is the JSON-RPC channel when the stdio transport is mounted |
| 275 | + // (#7915), so the notice must not be there — and under `os serve` |
| 276 | + // nothing else may be either. |
| 277 | + expect(stdout, `the drifted boot put bytes on stdout${seen}`).toBe(''); |
| 278 | + |
| 279 | + // ── The cost the notice exists to make visible ──────────────────── |
| 280 | + // The port the caller asked for is still answered by the stranger. This |
| 281 | + // is the false-green generator the card measured, re-derived here so the |
| 282 | + // notice is pinned against a real one rather than a described one. |
| 283 | + const answer = await fetch(`http://localhost:${neighbour.port}/api/v1/auth/sign-in/email`, { |
| 284 | + method: 'POST', |
| 285 | + }).then((r) => r.text()); |
| 286 | + expect(answer, 'the held port was not answered by the neighbour').toContain( |
| 287 | + 'A NEIGHBOURING AGENT DEV SERVER, not os serve', |
| 288 | + ); |
| 289 | + |
| 290 | + await stop(booted.child); |
| 291 | + } finally { |
| 292 | + await neighbour.release(); |
| 293 | + } |
| 294 | + }, 240_000); |
| 295 | + |
| 296 | + it('NEGATIVE — an ordinary boot on a free port says nothing about ports', async () => { |
| 297 | + const booted = await boot(randomPort()); |
| 298 | + const { stderr } = booted; |
| 299 | + const seen = `\n--- stderr (tail) ---\n${stderr.slice(-3000)}`; |
| 300 | + |
| 301 | + expect(booted.reachedBanner, `the boot never reached its banner${seen}`).toBe(true); |
| 302 | + expect(stderr, `the banner is missing, so "no notice" would prove nothing${seen}`).toContain( |
| 303 | + 'Server is ready', |
| 304 | + ); |
| 305 | + |
| 306 | + // ⭐ The whole value of the notice is that it is rare. The same regex the |
| 307 | + // positive case just matched must find nothing here. |
| 308 | + expect(stderr, `a drift notice appeared on a boot that never drifted${seen}`).not.toMatch( |
| 309 | + DRIFT_NOTICE, |
| 310 | + ); |
| 311 | + |
| 312 | + await stop(booted.child); |
| 313 | + }, 240_000); |
| 314 | +}); |
0 commit comments