From 7ab04d47675fe3935fd8f7310cfa6e170b352c31 Mon Sep 17 00:00:00 2001 From: Claude Code Date: Sat, 29 Aug 2026 10:37:53 +0000 Subject: [PATCH 1/2] fix(cli): write runtime..json before announcing the bound port os serve announced its address on the ready banner and the objectstack:listening IPC message BEFORE it wrote runtime..json, so every consumer that reacts to an announcement raced a file that did not exist yet. Publish through one ordered seam instead: publishBoundPort() drives the state file first, then IPC, then the banner. The channels are injected so the ORDER is observable, and a new deterministic test records the sequence and goes red if it is ever reversed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TvqBFLRzXdSPcbusDoED9k --- .changeset/serve-bound-port-publish-order.md | 33 +++ packages/cli/src/commands/serve.ts | 151 ++++++++++-- .../serve-bound-port-publish-order.test.ts | 222 ++++++++++++++++++ 3 files changed, 383 insertions(+), 23 deletions(-) create mode 100644 .changeset/serve-bound-port-publish-order.md create mode 100644 packages/cli/test/serve-bound-port-publish-order.test.ts diff --git a/.changeset/serve-bound-port-publish-order.md b/.changeset/serve-bound-port-publish-order.md new file mode 100644 index 0000000000..2971ad7a3d --- /dev/null +++ b/.changeset/serve-bound-port-publish-order.md @@ -0,0 +1,33 @@ +--- +"@objectstack/cli": patch +--- + +fix(cli): `os serve` writes `runtime..json` BEFORE it announces the port (#13193) + +`os serve` publishes the port it bound on three channels: the runtime state file +`runtime..json`, the `objectstack:listening` IPC message, and the +ready banner. Two of those ANNOUNCE an address; the third is the FILE those +consumers then open. + +FROM: the three fired in the order they had happened to be written — banner, +then IPC, then the file. Every consumer that believed an announcement therefore +raced a file that did not exist yet: a supervisor that opens +`runtime.env_local.json` when the banner says "ready", or an `os dev` parent +that reacts to the IPC message, could both reach the path before `serve` had +written it. Nothing errored on the producer side, so the window was invisible +from inside `serve`; a loaded machine simply widened it by descheduling the +child between the announcement and the write. + +TO: the state file is written first, and only then is the address announced — +IPC, then banner. A consumer that reacts to either announcement now finds the +file already on disk, by the producer's own program order rather than by luck. + +Unchanged: the three channels still publish the same bound port +(`resolveBoundPort`, #13062), each leg still keeps its own error handling, and a +state-file write that fails still cannot take the announcements down with it — +a boot does not die because a supervision file could not be written. + +The ordering is now a contract with a test that can observe it: +`publishBoundPort()` takes its three channels as arguments, and +`test/serve-bound-port-publish-order.test.ts` records the sequence and fails +deterministically if it is ever reversed. diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index d138ff34cf..f77ec4e802 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -433,6 +433,117 @@ export function resolveBoundPort( return reported; } +/** The `objectstack:listening` IPC message the `os dev` parent waits for. */ +export interface ListeningMessage { + type: 'objectstack:listening'; + port: number; + url: string; +} + +/** + * The three channels {@link publishBoundPort} drives — declared in the ONE + * order it is safe to drive them in, which is also the order the fields are + * listed here. + */ +export interface BoundPortChannels { + /** + * Writes `runtime..json`. ⛔ Must COMPLETE before either + * announcement below: it is the file both of them send a consumer to. + */ + writeRuntimeState: (published: { port: number; url: string }) => void; + /** Sends {@link ListeningMessage}, when an IPC channel is open. */ + announceListening: (message: ListeningMessage) => void; + /** Prints the ready banner, whose `API:` row names the same address. */ + printBanner: () => void; +} + +/** + * Publish the bound port on all three channels, in the one order that is safe. + * + * ## The bug this shape exists to make impossible + * + * `os serve` announces its address on three channels: the runtime state file + * `runtime..json`, the `objectstack:listening` IPC message, and + * the ready banner. Two of those are ANNOUNCEMENTS a consumer reacts to; the + * third is the FILE those consumers then open. Published in the order they + * happened to be written — banner, IPC, file — every consumer that believes an + * announcement races a file that is not there yet: + * + * ```text + * banner ─▶ a supervisor sees "ready" and opens runtime.env_local.json + * IPC ─▶ the `os dev` parent sees the port + * file ─────────────────────────▶ ...written here. The ENOENT already happened. + * ``` + * + * ⛔ Not hypothetical, and ⛔ not a test artefact. The e2e that pins #13062's + * claim (`serve-publishes-bound-port.e2e.test.ts`) is an ORDINARY consumer — it + * waits for the banner AND the IPC message, then reads the file — and it + * ejected 14 PRs from the shared merge queue in a rolling 24 hours (10 + * independent hits, #13158) with `ENOENT: ... runtime.env_local.json`. A real + * supervisor written the same way loses the same race; all a loaded machine + * does is deschedule the child between the announcement and the write, which is + * why it read as a flake for a day. + * + * ⛔ The repair is NOT to make the reader poll. A consumer that must poll after + * being told "ready" was told "ready" too early — polling spreads the defect + * into every consumer forever and hides it from the one place that can fix it. + * The producer owns the ordering: **write the file, THEN announce it.** + * + * ## Why the channels are injected rather than called inline + * + * The ORDER is the contract here, and an order is only pinned by a test that + * can observe it. Passing the channels in lets + * `serve-bound-port-publish-order.test.ts` record the sequence and go red + * DETERMINISTICALLY the day it is reversed. An end-to-end test cannot do that: + * it can only lose the race often enough for someone to notice — which is + * precisely the year-of-flakes this replaces. + */ +export function publishBoundPort(boundPort: number, channels: BoundPortChannels): void { + const url = `http://localhost:${boundPort}`; + // 1 ─ THE FILE FIRST. Both announcements below send a consumer to it. + channels.writeRuntimeState({ port: boundPort, url }); + // 2 ─ IPC: the `os dev` parent learns the real port without polling. + channels.announceListening({ type: 'objectstack:listening', port: boundPort, url }); + // 3 ─ The banner: a human, or a supervisor tailing stdout, reads the same + // address — and by now the state file it names is on disk. + channels.printBanner(); +} + +/** + * The real channels: the same three writes this command has always done, with + * their failure handling unchanged. + * + * Each leg keeps its OWN `try` — a boot must not die because a supervision file + * could not be written or because an IPC channel had already closed, and one + * leg failing must not cost the other two. Only the ORDER changed (#13193). + */ +export function runtimeBoundPortChannels(printBanner: () => void): BoundPortChannels { + return { + writeRuntimeState: ({ port, url }) => { + try { + const environmentId = process.env.OS_ENVIRONMENT_ID ?? 'env_local'; + const runtimeFile = path.join(resolveObjectStackHome(), `runtime.${environmentId}.json`); + fs.mkdirSync(path.dirname(runtimeFile), { recursive: true }); + fs.writeFileSync(runtimeFile, JSON.stringify({ + pid: process.pid, + port, + url, + environmentId, + startedAt: new Date().toISOString(), + }, null, 2)); + const cleanupRuntimeFile = () => { try { fs.rmSync(runtimeFile, { force: true }); } catch { /* noop */ } }; + process.on('exit', cleanupRuntimeFile); + } catch { /* non-fatal — supervision file is best-effort */ } + }, + announceListening: (message) => { + try { + if (typeof process.send === 'function') process.send(message); + } catch { /* IPC channel closed — best-effort */ } + }, + printBanner, + }; +} + /** * The IDENTITIES a capability provider registers under: full `plugin.name` ids * (`com.objectstack.mcp`) and/or exported class names (`MCPServerPlugin`). @@ -4289,7 +4400,14 @@ export default class Serve extends Command { // ── Clean startup summary ────────────────────────────────────── // #8978 — the Config:/Artifact: row must name what actually booted, // never `relativeConfig` unconditionally (see resolveBannerConfigRow). - printServerReady({ + // + // ⭐ A THUNK, not a call (#13193). The banner is one of the three + // bound-port channels, and {@link publishBoundPort} owns the order the + // three fire in — the state file has to be on disk before anything + // announces the address that names it. Nothing INSIDE this literal + // changed, and nothing in it is async, so deferring it to the call a few + // lines below is a pure move. + const printBanner = () => printServerReady({ // #10646 — the banner used to take `port` and compose // `http://localhost:` itself, which is where this process // LISTENS, not where an operator can reach it. On the EE 4.1.0 compose @@ -4363,30 +4481,17 @@ export default class Serve extends Command { // already right, because `getAvailablePort()` reassigned it; that is the // reading under which the old sentence looked true.) Publish the bound // one so supervisors and the `os dev` parent never have to guess: - // • IPC: when spawned with an 'ipc' channel (as `os dev` does), the - // parent learns the real port without polling. // • runtime.json: a small state file under OS_HOME for external // supervisors / health checks (pid + port + url). - const runtimeUrl = `http://localhost:${boundPort}`; - try { - if (typeof process.send === 'function') { - process.send({ type: 'objectstack:listening', port: boundPort, url: runtimeUrl }); - } - } catch { /* IPC channel closed — best-effort */ } - try { - const environmentId = process.env.OS_ENVIRONMENT_ID ?? 'env_local'; - const runtimeFile = path.join(resolveObjectStackHome(), `runtime.${environmentId}.json`); - fs.mkdirSync(path.dirname(runtimeFile), { recursive: true }); - fs.writeFileSync(runtimeFile, JSON.stringify({ - pid: process.pid, - port: boundPort, - url: runtimeUrl, - environmentId, - startedAt: new Date().toISOString(), - }, null, 2)); - const cleanupRuntimeFile = () => { try { fs.rmSync(runtimeFile, { force: true }); } catch { /* noop */ } }; - process.on('exit', cleanupRuntimeFile); - } catch { /* non-fatal — supervision file is best-effort */ } + // • IPC: when spawned with an 'ipc' channel (as `os dev` does), the + // parent learns the real port without polling. + // • the ready banner, whose `API:` row names the same address. + // + // ⭐ That list is in ORDER, and the order is the whole point (#13193): + // the file is written BEFORE either channel announces the address that + // sends a consumer to it. {@link publishBoundPort} carries the race the + // old order lost, and the reason the repair is not reader-side polling. + publishBoundPort(boundPort, runtimeBoundPortChannels(printBanner)); // Kernel already registers SIGINT/SIGTERM handlers during bootstrap. // No duplicate handler needed here — just keep the process alive. diff --git a/packages/cli/test/serve-bound-port-publish-order.test.ts b/packages/cli/test/serve-bound-port-publish-order.test.ts new file mode 100644 index 0000000000..7778387bb4 --- /dev/null +++ b/packages/cli/test/serve-bound-port-publish-order.test.ts @@ -0,0 +1,222 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #13193 / #13158 — the pin for the ORDER `os serve` publishes its bound port in. + * + * ## What broke, and why it read as a flake for a day + * + * `os serve` publishes the port it bound on three channels: the runtime state + * file `runtime..json`, the `objectstack:listening` IPC message, + * and the ready banner. Two of those ANNOUNCE an address; the third is the FILE + * a consumer then opens. `serve.ts` used to fire them banner → IPC → file, so + * every consumer that believed an announcement raced a file that did not exist + * yet. + * + * `serve-publishes-bound-port.e2e.test.ts` is an ordinary such consumer: it + * waits for the banner AND the IPC message, then reads the file. It ejected 14 + * PRs from the shared merge queue in a rolling 24 hours — 10 independent hits — + * always with the same reason line, never an assertion: + * + * ```text + * Error: ENOENT: no such file or directory, open '/tmp/os-bound-port-home-DEqSfV/runtime.env_local.json' + * ❯ channelsOf test/serve-publishes-bound-port.e2e.test.ts:241:28 + * ``` + * + * ⛔ The producer was wrong, not the reader. Load never created the race, it + * only widened it: all a busy machine does is deschedule the child between the + * announcement and the write. + * + * ## Why this file exists ALONGSIDE that e2e, rather than instead of it + * + * ⭐ An end-to-end test cannot pin an ordering. It can only lose the race often + * enough for someone to notice — which is the failure mode being repaired, and + * a "fix" verified only by that e2e passing once would be indistinguishable + * from a fix that did nothing. So the ordering is observed DIRECTLY here: + * `publishBoundPort` takes its three channels as arguments, and these tests + * record the sequence it drives them in. Reverse the order in `serve.ts` and + * this file goes red on every machine, every run, with no load required. + * + * The e2e keeps its own job — proving the three channels agree with the socket + * (#13062). This file proves they cannot be announced before they are true. + * + * ## ⛔ Why the real IPC leg is never CALLED here + * + * `runtimeBoundPortChannels().announceListening` is `process.send`, and under + * vitest's `forks` pool `process.send` is the runner's OWN control channel. A + * test that drove that leg for real would post `objectstack:listening` to + * vitest itself. The real leg's body is one guarded `process.send`; what is + * worth pinning about it — WHEN it fires — is pinned by observing the moment, + * not by delivering the message. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { + publishBoundPort, + runtimeBoundPortChannels, + type BoundPortChannels, + type ListeningMessage, +} from '../src/commands/serve.js'; + +/** The file name a plain `os serve` writes when `OS_ENVIRONMENT_ID` is unset. */ +const RUNTIME_FILE = 'runtime.env_local.json'; + +const tempDirs: string[] = []; +/** `runtimeBoundPortChannels` registers an `exit` cleanup per state file written. */ +const exitListenersBefore = process.listeners('exit').slice(); + +afterEach(() => { + for (const listener of process.listeners('exit')) { + if (!exitListenersBefore.includes(listener)) process.removeListener('exit', listener); + } + while (tempDirs.length) { + const dir = tempDirs.pop() as string; + try { rmSync(dir, { recursive: true, force: true }); } catch { /* best effort */ } + } +}); + +/** A temp directory, torn down by `afterEach`. */ +function tempDir(): string { + const dir = mkdtempSync(join(tmpdir(), 'os-publish-order-')); + tempDirs.push(dir); + return dir; +} + +/** + * Run `fn` with `OS_HOME` pointed at `home` and `OS_ENVIRONMENT_ID` unset, then + * put both back exactly as they were — including "was not set at all", which a + * bare reassignment cannot express. + */ +function withHome(home: string, fn: () => T): T { + const priorHome = process.env.OS_HOME; + const priorEnvId = process.env.OS_ENVIRONMENT_ID; + process.env.OS_HOME = home; + delete process.env.OS_ENVIRONMENT_ID; + try { + return fn(); + } finally { + if (priorHome === undefined) delete process.env.OS_HOME; else process.env.OS_HOME = priorHome; + if (priorEnvId === undefined) delete process.env.OS_ENVIRONMENT_ID; else process.env.OS_ENVIRONMENT_ID = priorEnvId; + } +} + +describe('#13193 — `os serve` writes the state file BEFORE it announces the port', () => { + it('drives the three channels in the order state-file → IPC → banner', () => { + const order: string[] = []; + + publishBoundPort(45671, { + writeRuntimeState: () => { order.push('state-file'); }, + announceListening: () => { order.push('ipc'); }, + printBanner: () => { order.push('banner'); }, + } satisfies BoundPortChannels); + + // ⛔ Not `order[0] === 'state-file'` alone: a publish that silently stopped + // announcing would satisfy that, and losing a channel is the defect in the + // opposite direction — the one #13062 was filed for. + expect(order, 'all three channels must fire, in this exact order').toEqual([ + 'state-file', 'ipc', 'banner', + ]); + }); + + it('gives both announcements the same address the state file was written with', () => { + let written: { port: number; url: string } | undefined; + let announced: ListeningMessage | undefined; + let bannerPrinted = false; + + publishBoundPort(45672, { + writeRuntimeState: (published) => { written = published; }, + announceListening: (message) => { announced = message; }, + printBanner: () => { bannerPrinted = true; }, + }); + + expect(written).toEqual({ port: 45672, url: 'http://localhost:45672' }); + expect(announced).toEqual({ + type: 'objectstack:listening', + port: 45672, + url: 'http://localhost:45672', + }); + expect(bannerPrinted).toBe(true); + }); + + /** + * ⭐ The load-bearing one, and the reason this file drives the REAL writer + * rather than three recorders. + * + * The e2e's failure is `existsSync(...) === false` at the instant it reacts to + * an announcement. So that is measured here at the instant each announcement + * FIRES — not after the publish returns, where the ordering under test has + * already finished and both orders look identical. Against the old order both + * booleans below are `false` deterministically; against the new one both are + * `true` deterministically. No load, no sleep, no retry. + */ + it('has the state file already on disk at the moment each announcement fires', () => { + const home = tempDir(); + const runtimeFile = join(home, RUNTIME_FILE); + + let existedAtIpc: boolean | undefined; + let existedAtBanner: boolean | undefined; + + withHome(home, () => { + expect(existsSync(runtimeFile), 'precondition: no state file before the publish').toBe(false); + + // The REAL writer — the code path `os serve` runs. Only the two + // announcement legs are observers (see the header for why). + const real = runtimeBoundPortChannels(() => { + existedAtBanner = existsSync(runtimeFile); + }); + + publishBoundPort(45673, { + writeRuntimeState: real.writeRuntimeState, + announceListening: () => { existedAtIpc = existsSync(runtimeFile); }, + printBanner: real.printBanner, + }); + }); + + expect(existedAtIpc, 'runtime state file must exist when the IPC message is sent').toBe(true); + expect(existedAtBanner, 'runtime state file must exist when the banner prints').toBe(true); + + // And it must already carry the BOUND port — not a placeholder written + // early to win an ordering check. Present is only half the contract. + const state = JSON.parse(readFileSync(runtimeFile, 'utf8')); + expect(state.port).toBe(45673); + expect(state.url).toBe('http://localhost:45673'); + expect(state.environmentId).toBe('env_local'); + expect(state.pid).toBe(process.pid); + }); + + /** + * Moving the write to the FRONT put it upstream of both announcements, so a + * write that fails must not be able to take them down with it. It cannot: + * the real writer swallows its own failure, exactly as it did when it ran + * last. A boot does not die because a supervision file could not be written. + */ + it('still announces when the real state-file write fails', () => { + // `OS_HOME` under a regular FILE — `mkdirSync(..., {recursive:true})` + // fails with ENOTDIR, deterministically and with no permissions games. + const blocker = join(tempDir(), 'not-a-directory'); + writeFileSync(blocker, 'x'); + const home = join(blocker, 'nested'); + + const order: string[] = []; + + withHome(home, () => { + const real = runtimeBoundPortChannels(() => { order.push('banner'); }); + publishBoundPort(45674, { + writeRuntimeState: (published) => { + order.push('state-file'); + real.writeRuntimeState(published); + }, + announceListening: () => { order.push('ipc'); }, + printBanner: real.printBanner, + }); + }); + + expect(order, 'a failed write must not swallow the announcements').toEqual([ + 'state-file', 'ipc', 'banner', + ]); + expect(existsSync(join(home, RUNTIME_FILE)), 'the write really did fail').toBe(false); + }); +}); From 35574c9dbb7aff8559f055961f96b44471be8b63 Mon Sep 17 00:00:00 2001 From: Claude Code Date: Sat, 29 Aug 2026 12:28:22 +0000 Subject: [PATCH 2/2] test(cli): rewrite #13062's publish pins against the seam, behaviourally The three pins that read serve.ts source text for the IPC call, the state-file literal and const runtimeUrl went red when #13193 folded those three publish sites into publishBoundPort(). The behaviour they guarded is intact, so they are rewritten UPWARD rather than relaxed: two channels are now driven through the seam and observed, the state file is read off disk (pid included), and the IPC leg is observed reaching process.send. The wiring half stays a source pin because run() is still un-enterable in process - but it is now one call site instead of three publish sites. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TvqBFLRzXdSPcbusDoED9k --- .../serve-bound-port-publication.test.ts | 186 ++++++++++++++++-- 1 file changed, 166 insertions(+), 20 deletions(-) diff --git a/packages/cli/src/commands/serve-bound-port-publication.test.ts b/packages/cli/src/commands/serve-bound-port-publication.test.ts index c413015981..031a6626fd 100644 --- a/packages/cli/src/commands/serve-bound-port-publication.test.ts +++ b/packages/cli/src/commands/serve-bound-port-publication.test.ts @@ -34,9 +34,10 @@ * names which channel regressed.) */ -import { describe, it, expect } from 'vitest'; -import { readFileSync } from 'node:fs'; -import { resolve } from 'node:path'; +import { describe, it, expect, afterEach } from 'vitest'; +import { existsSync, mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; // The repo's ONE code/prose separator, typed by the hand-written `.d.mts` @@ -45,7 +46,12 @@ import { fileURLToPath } from 'node:url'; // port", and a comment claiming it does is precisely what was there before. import { maskComments } from '../../../../scripts/js-comment-mask.mjs'; -import { resolveBoundPort } from './serve.js'; +import { + publishBoundPort, + resolveBoundPort, + runtimeBoundPortChannels, + type BoundPortChannels, +} from './serve.js'; import { MAX_PORT } from '../utils/port-contract.js'; /** …/packages/cli/src/commands — seeded from `import.meta.url`. */ @@ -175,21 +181,146 @@ describe('#13062 resolveBoundPort — the transport answers, not the request', ( }); }); -describe('#13062 all THREE channels publish that one number — read off the code', () => { - it('the `objectstack:listening` IPC message', () => { - expect( - SERVE, - 'the IPC message no longer publishes `boundPort` — `os dev` reads this channel to learn ' - + 'where its child ended up, and #13061 records that `os start` could read it too', - ).toContain("process.send({ type: 'objectstack:listening', port: boundPort, url: runtimeUrl });"); +/** Temp `OS_HOME` directories made by the behavioural pins below. */ +const publishHomes: string[] = []; +/** `runtimeBoundPortChannels` registers one `exit` cleanup per state file written. */ +const exitListenersAtLoad = process.listeners('exit').slice(); + +afterEach(() => { + for (const listener of process.listeners('exit')) { + if (!exitListenersAtLoad.includes(listener)) process.removeListener('exit', listener); + } + while (publishHomes.length) { + const home = publishHomes.pop() as string; + try { rmSync(home, { recursive: true, force: true }); } catch { /* best effort */ } + } +}); + +/** + * Run `fn` with `OS_HOME` at a fresh temp dir and `OS_ENVIRONMENT_ID` unset, + * then restore both — including "was not set at all", which a bare reassignment + * cannot express. Returns the directory so the caller can read what was written. + */ +function withTempHome(fn: (home: string) => T): { home: string; value: T } { + const home = mkdtempSync(join(tmpdir(), 'os-bound-port-publication-')); + publishHomes.push(home); + const priorHome = process.env.OS_HOME; + const priorEnvId = process.env.OS_ENVIRONMENT_ID; + process.env.OS_HOME = home; + delete process.env.OS_ENVIRONMENT_ID; + try { + return { home, value: fn(home) }; + } finally { + if (priorHome === undefined) delete process.env.OS_HOME; else process.env.OS_HOME = priorHome; + if (priorEnvId === undefined) delete process.env.OS_ENVIRONMENT_ID; else process.env.OS_ENVIRONMENT_ID = priorEnvId; + } +} + +/** + * Drive `fn` with `process.send` replaced by a recorder, and hand back what it + * was given. + * + * ⚠️ Under vitest's `forks` pool `process.send` is the RUNNER's own control + * channel, so this must never deliver a real `objectstack:listening` message to + * it. The swap is synchronous, spans one call, and is undone in `finally`. + */ +function recordingProcessSend(fn: () => void): unknown[] { + const sent: unknown[] = []; + const prior = process.send; + (process as { send?: unknown }).send = (message: unknown) => { sent.push(message); return true; }; + try { fn(); } finally { (process as { send?: unknown }).send = prior; } + return sent; +} + +/** + * #13062: all three channels publish ONE number, and it is the BOUND one. + * + * ## Why three of these are DRIVEN now, where they used to be source greps + * + * They read the source because `run()` is one ~3000-line method needing a whole + * kernel to enter, so nothing in-process could observe what its three publish + * sites read. #13193 changed the shape: the publish is now one exported seam, + * {@link publishBoundPort}, that takes its three channels as ARGUMENTS. So + * "all three publish that one number" is now driven and observed instead of + * grepped — strictly stronger, because a grep passes on text that never runs, + * and it survives the next refactor of the same code. + * + * ⛔ What did NOT become reachable is the WIRING question — whether the seam is + * handed `boundPort` or `port` at its call site inside `run()`. That is still + * un-enterable in-process, so it stays a source pin, and it is a better one + * than before: there is now exactly ONE site to get wrong instead of three. + * + * ⛔ The ORDER the seam drives the three in is #13193's property, pinned in + * `test/serve-bound-port-publish-order.test.ts`. Kept separate deliberately — + * these two files fail for different reasons and should keep naming them. + */ +describe('#13062 all THREE channels publish that one number', () => { + it('hands the SAME bound number to the state file and the IPC message', () => { + let written: { port: number; url: string } | undefined; + let announced: { type?: string; port?: unknown; url?: unknown } | undefined; + let banners = 0; + + publishBoundPort(45062, { + writeRuntimeState: (published) => { written = published; }, + announceListening: (message) => { announced = message; }, + printBanner: () => { banners += 1; }, + } satisfies BoundPortChannels); + + // ⛔ Not "each carries a port" — that passes for the very defect #13062 + // fixed, where all three agreed on the REQUESTED number. The assertion is + // that both carry THE SAME one, and that the URL is composed from it. + expect(written).toEqual({ port: 45062, url: 'http://localhost:45062' }); + expect(announced).toEqual({ + type: 'objectstack:listening', + port: 45062, + url: 'http://localhost:45062', + }); + expect(announced?.port, 'the two channels disagree').toBe(written?.port); + expect(announced?.url).toBe(written?.url); + expect(banners, 'the third channel was not driven at all').toBe(1); }); - it('`runtime..json`', () => { - expect( - SERVE, - 'the runtime state file no longer publishes `boundPort` — external supervisors and health ' - + 'checks read it for the address to poll', - ).toMatch(/pid: process\.pid,\s*\n\s*port: boundPort,/); + it('`runtime..json` really lands, carrying `pid` beside that port', () => { + // The supervisor contract this file has always guarded, now read off the + // FILE instead of off a regex about how the object literal is formatted. + const { home } = withTempHome(() => { + publishBoundPort(45063, runtimeBoundPortChannels(() => { /* banner not under test here */ })); + }); + + const runtimeFile = join(home, 'runtime.env_local.json'); + expect(existsSync(runtimeFile), 'no runtime state file was written at all').toBe(true); + const state = JSON.parse(readFileSync(runtimeFile, 'utf8')); + expect(state.port, 'the state file does not publish the bound port').toBe(45063); + expect(state.url).toBe('http://localhost:45063'); + expect(state.pid, 'external supervisors read `pid` beside the port').toBe(process.pid); + expect(state.environmentId).toBe('env_local'); + }); + + it('the `objectstack:listening` IPC message really reaches `process.send`', () => { + // `os dev` reads this channel to learn where its child ended up, and #13061 + // records that `os start` could read it too — so the leg is pinned by + // OBSERVING the send, not by grepping for the call. + const channels = runtimeBoundPortChannels(() => { /* banner not under test here */ }); + const sent = recordingProcessSend(() => { + channels.announceListening({ type: 'objectstack:listening', port: 45064, url: 'http://localhost:45064' }); + }); + + expect(sent).toEqual([{ type: 'objectstack:listening', port: 45064, url: 'http://localhost:45064' }]); + }); + + it('and stays silent, rather than throwing, when no IPC channel is open', () => { + // The ordinary `os serve` case: no parent, no fd 3. A publish that threw + // here would take the banner and the state file down with it. + const channels = runtimeBoundPortChannels(() => { /* unused */ }); + const prior = process.send; + (process as { send?: unknown }).send = undefined; + try { + expect(() => channels.announceListening({ + type: 'objectstack:listening', port: 45065, url: 'http://localhost:45065', + })).not.toThrow(); + } finally { + (process as { send?: unknown }).send = prior; + } }); it('the ready banner, through the runtime\'s own base-URL chain', () => { @@ -199,8 +330,19 @@ describe('#13062 all THREE channels publish that one number — read off the cod ).toContain('externalBaseOrigin: resolveAuthBaseUrl(boundPort).baseOrigin'); }); - it('the URL those channels carry is composed from the same number', () => { - expect(SERVE).toContain('const runtimeUrl = `http://localhost:${boundPort}`;'); + it('the ONE wiring site hands the seam the BOUND port, never the requested one', () => { + // The half that cannot be driven in-process, and the whole of what is left + // of the source scan for these channels: `run()` reaches the seam once, and + // what it passes decides all three channels at once. + expect( + SERVE, + 'the publish site no longer hands `publishBoundPort` the resolved bound port', + ).toContain('publishBoundPort(boundPort, runtimeBoundPortChannels(printBanner));'); + // Exactly two mentions in CODE: the declaration and that single call. + expect( + SERVE.match(/publishBoundPort\(/g) ?? [], + 'a second publish site can disagree with the first — that is the #13062 defect returning', + ).toHaveLength(2); }); it('⛔ and NONE of the three has drifted back onto the requested port', () => { @@ -208,7 +350,11 @@ describe('#13062 all THREE channels publish that one number — read off the cod // one leaves two lying in a place nobody thinks to look next time. expect(SERVE).not.toContain('port: Number(port)'); expect(SERVE).not.toContain('externalBaseOrigin: resolveAuthBaseUrl(port)'); - expect(SERVE).not.toContain('const runtimeUrl = `http://localhost:${port}`'); + // ⛔ `const runtimeUrl = ...` is gone (#13193 folded it into the seam), so a + // negative naming it would pass for the wrong reason. The live spelling of + // the same regression is the seam being handed the REQUESTED port. + expect(SERVE).not.toContain('publishBoundPort(port,'); + expect(SERVE).not.toContain('publishBoundPort(Number(port)'); }); it('resolves it ONCE, from the transport, after the boot', () => {