Skip to content

Commit 8caf3d0

Browse files
committed
fix(cli): os dev's serve child announces objectstack:listening after its banner
The os dev parent prints its MCP connect block when the serve child's objectstack:listening message arrives, into the terminal the child prints its ready banner to. The child sent the message before the banner, so the two processes wrote one terminal at once and the block interleaved with the banner (#22410). publishBoundPort now drives state file, then banner, then IPC. The banner is written synchronously to a terminal, so the parent's block prints under a finished banner: a causal order, no timer. Claude-Session: https://claude.ai/code/session_01BmsuLyUeuG5CNpZFMH1jzS Co-authored-by: Claude <noreply@anthropic.com>
1 parent 821d6a5 commit 8caf3d0

3 files changed

Lines changed: 111 additions & 37 deletions

File tree

‎packages/cli/src/commands/serve.ts‎

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -718,10 +718,15 @@ export interface BoundPortChannels {
718718
* either announcement below: it is the file both of them send a consumer to.
719719
*/
720720
writeRuntimeState: (published: { port: number; url: string }) => void;
721-
/** Sends {@link ListeningMessage}, when an IPC channel is open. */
722-
announceListening: (message: ListeningMessage) => void;
723-
/** Prints the ready banner, whose `API:` row names the same address. */
721+
/**
722+
* Prints the ready banner, whose `API:` row names the same address, and the
723+
* boot diagnostics under it. ⛔ Must COMPLETE before the IPC message below:
724+
* the `os dev` parent prints its MCP connect block on that message, into the
725+
* same terminal (#22410).
726+
*/
724727
printBanner: () => void;
728+
/** Sends {@link ListeningMessage}, when an IPC channel is open. LAST — see above. */
729+
announceListening: (message: ListeningMessage) => void;
725730
}
726731

727732
/**
@@ -757,6 +762,25 @@ export interface BoundPortChannels {
757762
* into every consumer forever and hides it from the one place that can fix it.
758763
* The producer owns the ordering: **write the file, THEN announce it.**
759764
*
765+
* ## The second race: two processes, one terminal (#22410)
766+
*
767+
* The IPC message is not only a fact for a program. The `os dev` parent prints
768+
* its MCP connect block (`🤖 MCP server — connect a coding agent:` and its
769+
* Endpoint / Skill / Connect / Disable rows) the moment the message arrives,
770+
* into the terminal this process writes its banner to. Sent BEFORE the banner,
771+
* the two processes wrote that terminal at once: measured under a pty on the
772+
* Build-with-Claude-Code tutorial project, 6 of 7 boots printed the block
773+
* above or inside the banner, 2 of them with banner rows inside the block, and
774+
* one probe boot printed the whole credential and plugin section between
775+
* `Skill` and `Connect`.
776+
*
777+
* So the banner comes SECOND and the message LAST. That makes the order
778+
* causal, not timed: the banner is written synchronously (a terminal is
779+
* synchronous stdio on POSIX, and `keepStderrNonBlocking` leaves a TTY
780+
* alone), so every banner byte is in the terminal before the message is sent,
781+
* and the parent prints only on receipt. ⛔ No sleep or timer orders anything
782+
* here. The message's shape is unchanged; it arrives a banner's print later.
783+
*
760784
* ## Why the channels are injected rather than called inline
761785
*
762786
* The ORDER is the contract here, and an order is only pinned by a test that
@@ -783,11 +807,12 @@ export function publishBoundPort(
783807
const url = `${boundProtocol}://localhost:${boundPort}`;
784808
// 1 ─ THE FILE FIRST. Both announcements below send a consumer to it.
785809
channels.writeRuntimeState({ port: boundPort, url });
786-
// 2 ─ IPC: the `os dev` parent learns the real port without polling.
787-
channels.announceListening({ type: 'objectstack:listening', port: boundPort, url });
788-
// 3 ─ The banner: a human, or a supervisor tailing stdout, reads the same
810+
// 2 ─ The banner: a human, or a supervisor tailing stdout, reads the same
789811
// address — and by now the state file it names is on disk.
790812
channels.printBanner();
813+
// 3 ─ IPC LAST: the `os dev` parent learns the real port without polling,
814+
// and prints its MCP connect block under a banner that has finished (#22410).
815+
channels.announceListening({ type: 'objectstack:listening', port: boundPort, url });
791816
}
792817

793818
/**
@@ -5312,14 +5337,16 @@ export default class Serve extends Command {
53125337
// one so supervisors and the `os dev` parent never have to guess:
53135338
// • runtime.json: a small state file under OS_HOME for external
53145339
// supervisors / health checks (pid + port + url).
5340+
// • the ready banner, whose `API:` row names the same address.
53155341
// • IPC: when spawned with an 'ipc' channel (as `os dev` does), the
53165342
// parent learns the real port without polling.
5317-
// • the ready banner, whose `API:` row names the same address.
53185343
//
53195344
// ⭐ That list is in ORDER, and the order is the whole point (commit faff497fd):
53205345
// the file is written BEFORE either channel announces the address that
5321-
// sends a consumer to it. {@link publishBoundPort} carries the race the
5322-
// old order lost, and the reason the repair is not reader-side polling.
5346+
// sends a consumer to it, and the banner is printed whole BEFORE the
5347+
// message that makes the `os dev` parent print into the same terminal
5348+
// (#22410). {@link publishBoundPort} carries both races the old orders
5349+
// lost, and the reason neither repair is a reader-side poll or a timer.
53235350
publishBoundPort(boundPort, runtimeBoundPortChannels(printBanner), boundProtocol);
53245351

53255352
// ── …and one beat later, whether the APP is ready too (#17329) ──

‎packages/cli/test/dev-boot-output-order.integration.test.ts‎

Lines changed: 62 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,12 @@ const ALL_BOOTS_TIMEOUT_MS = BOOTS * (BOOT_TIMEOUT_MS + BLOCK_GRACE_MS + 30_000)
7979

8080
const SCAFFOLD = {
8181
manifest: { id: 'com.example.order', namespace: 'order', version: '1.0.0', type: 'app', name: 'order' },
82-
objects: [{ name: 'order_item', label: 'Item', fields: { title: { type: 'text', label: 'Title' } } }],
82+
objects: [{
83+
name: 'order_item',
84+
label: 'Item',
85+
sharingModel: 'private',
86+
fields: { title: { type: 'text', label: 'Title' } },
87+
}],
8388
};
8489

8590
const ESC = String.fromCharCode(27);
@@ -217,40 +222,72 @@ function recorded(): BootReading[] {
217222
return readings as BootReading[];
218223
}
219224

220-
/** Where the block's header is, and what printed in the four lines under it. */
221-
function blockOf(r: BootReading): { header: number; under: string[] } {
225+
/** One boot's reading of the screen: where the block starts and ends, and what printed inside it. */
226+
interface ScreenReading {
227+
boot: number;
228+
/** The block header's line, or -1. */
229+
header: number;
230+
/** The block's last row (`Disable`), or -1. */
231+
disable: number;
232+
/** Lines between the header and the last row that are not the block's own rows. */
233+
foreign: string[];
234+
/** The `🔑 Dev admin` line, or -1. */
235+
credential: number;
236+
/** The banner's last row (`Press Ctrl+C to stop`), or -1. */
237+
tail: number;
238+
}
239+
240+
function readScreen(r: BootReading): ScreenReading {
222241
const header = r.lines.findIndex((l) => l.includes(BLOCK_HEADER));
223-
expect(header, `boot ${r.boot}: no MCP connect block on the screen`).toBeGreaterThan(-1);
224-
return { header, under: r.lines.slice(header + 1, header + 1 + BLOCK_ROWS.length) };
242+
const disable = header < 0 ? -1 : r.lines.findIndex((l, i) => i > header && BLOCK_ROWS[3].test(l));
243+
const foreign = header < 0 || disable < 0
244+
? []
245+
: r.lines.slice(header + 1, disable).filter((l) => !BLOCK_ROWS.some((row) => row.test(l))).map((l) => l.trim());
246+
return {
247+
boot: r.boot,
248+
header,
249+
disable,
250+
foreign,
251+
credential: r.lines.findIndex((l) => l.includes(CREDENTIAL)),
252+
tail: r.lines.findIndex((l) => l.includes(BANNER_TAIL)),
253+
};
254+
}
255+
256+
/**
257+
* Every boot's reading, after the control: each boot printed the whole block,
258+
* its banner tail and its credential line, so every verdict below is a reading
259+
* of something that is there rather than of an absence.
260+
*/
261+
function screens(): ScreenReading[] {
262+
const all = recorded().map(readScreen);
263+
const absent = all.flatMap((s) => [
264+
...(s.header < 0 || s.disable < 0 ? [`boot ${s.boot}: no whole MCP connect block on the screen`] : []),
265+
...(s.tail < 0 ? [`boot ${s.boot}: no banner tail`] : []),
266+
...(s.credential < 0 ? [`boot ${s.boot}: no credential line — the boot did not seed its dev admin`] : []),
267+
]);
268+
expect(absent, 'the control: every boot prints the block, the banner and the credential line').toEqual([]);
269+
return all;
225270
}
226271

227272
describe('#22410 — `os dev` prints the banner, then the MCP block, each block whole', () => {
228273
it('the MCP block\'s rows are contiguous on every boot', () => {
229-
for (const r of recorded()) {
230-
const { under } = blockOf(r);
231-
under.forEach((line, i) => {
232-
expect(line, `boot ${r.boot}: row ${i + 1} under the block header is not the block's own`).toMatch(BLOCK_ROWS[i]);
233-
});
234-
}
274+
const split = screens()
275+
.filter((s) => s.disable - s.header !== BLOCK_ROWS.length || s.foreign.length > 0)
276+
.map((s) => `boot ${s.boot}: ${s.foreign.length} foreign line(s) inside the block: ${JSON.stringify(s.foreign)}`);
277+
expect(split, `${split.length} of ${BOOTS} boots split the block`).toEqual([]);
235278
});
236279

237280
it('the dev admin credential line sits outside the block on every boot', () => {
238-
for (const r of recorded()) {
239-
const { header } = blockOf(r);
240-
const credential = r.lines.findIndex((l) => l.includes(CREDENTIAL));
241-
// Present, so "outside" is a reading and not an absence.
242-
expect(credential, `boot ${r.boot}: no credential line — the boot did not seed its dev admin`).toBeGreaterThan(-1);
243-
const inside = credential > header && credential <= header + BLOCK_ROWS.length;
244-
expect(inside, `boot ${r.boot}: the credential line printed inside the MCP block`).toBe(false);
245-
}
281+
const inside = screens()
282+
.filter((s) => s.credential > s.header && s.credential < s.disable)
283+
.map((s) => `boot ${s.boot}: the credential line printed inside the MCP block`);
284+
expect(inside, `${inside.length} of ${BOOTS} boots printed the credential inside the block`).toEqual([]);
246285
});
247286

248287
it('the block comes after the whole banner on every boot — one fixed order', () => {
249-
for (const r of recorded()) {
250-
const { header } = blockOf(r);
251-
const tail = r.lines.findIndex((l) => l.includes(BANNER_TAIL));
252-
expect(tail, `boot ${r.boot}: no banner tail`).toBeGreaterThan(-1);
253-
expect(header, `boot ${r.boot}: the MCP block started before the banner finished`).toBeGreaterThan(tail);
254-
}
288+
const early = screens()
289+
.filter((s) => s.header < s.tail)
290+
.map((s) => `boot ${s.boot}: the MCP block started before the banner finished`);
291+
expect(early, `${early.length} of ${BOOTS} boots printed the block inside or above the banner`).toEqual([]);
255292
});
256293
});

‎packages/cli/test/serve-bound-port-publish-order.test.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,16 @@
3939
* The e2e keeps its own job — proving the three channels agree with the socket
4040
* (#13062). This file proves they cannot be announced before they are true.
4141
*
42+
* ## The second constraint on the same sequence (#22410)
43+
*
44+
* The banner comes BEFORE the IPC message. The `os dev` parent prints its MCP
45+
* connect block when `objectstack:listening` arrives, into the terminal the
46+
* child prints its banner to; sent first, the message let the two processes
47+
* write that terminal at once, and the block's rows came out interleaved with
48+
* the banner's. The full order is therefore state-file → banner → IPC, and
49+
* what a person sees under a pty is pinned by
50+
* `dev-boot-output-order.integration.test.ts`.
51+
*
4252
* ## ⛔ Why the real IPC leg is never CALLED here
4353
*
4454
* `runtimeBoundPortChannels().announceListening` is `process.send`, and under
@@ -112,7 +122,7 @@ function withHome<T>(home: string, fn: () => T): T {
112122
}
113123

114124
describe('#13193 — `os serve` writes the state file BEFORE it announces the port', () => {
115-
it('drives the three channels in the order state-file → IPC → banner', () => {
125+
it('drives the three channels in the order state-file → banner → IPC', () => {
116126
const order: string[] = [];
117127

118128
publishBoundPort(45671, {
@@ -125,7 +135,7 @@ describe('#13193 — `os serve` writes the state file BEFORE it announces the po
125135
// announcing would satisfy that, and losing a channel is the defect in the
126136
// opposite direction — the one #13062 was filed for.
127137
expect(order, 'all three channels must fire, in this exact order').toEqual([
128-
'state-file', 'ipc', 'banner',
138+
'state-file', 'banner', 'ipc',
129139
]);
130140
});
131141

@@ -223,7 +233,7 @@ describe('#13193 — `os serve` writes the state file BEFORE it announces the po
223233
});
224234

225235
expect(order, 'a failed write must not swallow the announcements').toEqual([
226-
'state-file', 'ipc', 'banner',
236+
'state-file', 'banner', 'ipc',
227237
]);
228238
expect(existsSync(join(home, RUNTIME_FILE)), 'the write really did fail').toBe(false);
229239
});

0 commit comments

Comments
 (0)