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-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', () => { 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); + }); +});