Skip to content

Commit b7f29dd

Browse files
fix(cli): os serve with the stdio MCP transport and no config argument answers an initialize written at spawn — its config positional no longer reads stdin (#24212)
Fixes #24191 Clause-②: no `os serve` with the stdio MCP transport enabled and no config argument now answers the `initialize` an MCP client writes when it spawns the server, and it boots the configured app. ## What was wrong (re-measured on `origin/main` `f963d8e2b`, built CLI through `bin/run.js`) - **H1 holds.** `serve`'s `config` positional (the `Args.string` at about `serve.ts:1400`) had no `ignoreStdin`. In `@oclif/core` 5.1.2 (`lib/parser/parse.js`), `_args()` calls `tryStdin` for every positional that has no token, unless the arg sets `ignoreStdin`. A line already waiting in the pipe becomes the value. - The fixture is the shape the card names: a host config (`defineStack` plus a plugin that writes a marker from `start()`), a compiled `dist/objectstack.json` beside it, a key minted through `serve --dev` and `POST /keys`, and `os serve` with `OS_MCP_STDIO_ENABLED=true`, that key and `NODE_ENV` unset (production). The `initialize` is written at spawn in both rows. | shape | before (`f963d8e2b`) | after | |---|---|---| | `os serve -p PORT --no-ui` (no config argument) | no answer within 60 s; `No objectstack.config.ts found — booting from artifact (default host)...` beside the config; host plugin marker absent | answered (`id` 24191); host plugin marker present | | control: `os serve objectstack.config.ts -p PORT --no-ui` | answered, config loaded (1 passed) | unchanged | ## The fix (`packages/cli/src/commands/serve.ts`) `ignoreStdin: true` on `serve`'s `config` positional. Stdin belongs to the stdio transport; the config path comes from argv or the default. The fix sits at the producer, the parser declaration that let a protocol message become a config path. Nothing downstream changes, and no consumer gains a fallback. The one input-surface change: an omitted config argument is no longer read from stdin. **H2 holds** on this tree: - a config piped into `serve`, `start` or `compile` (a pipe into `os`, `objectstack` or `bin/run(-dev).js` followed by one of those commands): **0** hits; - a stdin redirect into those commands: **0** real hits (the 6 regex hits are prose placeholders such as a bracketed "config dir" after `os serve`); - `stdin.write(` / `stdin.end(` under `packages/cli`: **5** hits, 3 login prompts and 2 JSON-RPC `initialize` writes, none of them a config path. The changeset states the change and its remedy: pass the path as the argument. ## Re-grounding `serve-mcp-stdio-answers.e2e.test.ts` (#7645), not dropping it **H3 holds, measured.** That file spawns `serve` with no positional because oclif's `tryStdin` used to pause stdin, and a paused stdin is the premise of the `resume()` in `MCPServerRuntime.start()`. After this fix nothing in the spawned CLI pauses stdin, so the premise was gone: | e2e file | `resume()` removed (local ablation of `packages/mcp`, rebuilt, marker in 2 built files) | `resume()` restored | |---|---|---| | as on `f963d8e2b` | **passed**: vacuous, it no longer guards `resume()` | passed | | re-grounded (this PR) | **RED**: `the stdio transport started but never answered initialize`, after its premise assertion passed | passed | The re-grounded stdio boot runs with a `--import` preload, a `data:` URL so the whole host stays on the page. The preload is the "host that touched stdin before `start()`" that the `resume()` comment names. It pauses `process.stdin` before the CLI loads, which is what readline's `Interface.close()` did under oclif. It also prints `readableFlowing` each time a `data` listener attaches. The test asserts every reported state is `false`, so the premise is measured on every run. A later change that resumes stdin earlier cannot make the file vacuous again without reddening it. The file still passes no positional, the form users type, and still asserts a parsed answer. The `packages/mcp` ablation was local only. It was restored through `ablation-replace.mjs` (blob `3ef6ce641` equals HEAD), the rebuilt `dist/` no longer carries the marker (6 files), and the tree is clean. The file is nightly-tier by name, so this PR's CI does not run it. The readings above are local. ## Pin `packages/cli/test/serve-mcp-stdio-early-initialize.integration.test.ts` is in the `integration` tier, because it spawns `bin/run.js`. It runs one minting `serve --dev` boot, one `os compile` for the artifact, and then the boot under test with no config argument and the `initialize` written at spawn. It asserts: 1. A parseable JSON-RPC frame with the request's own `id` (24191), no `error`, a `protocolVersion` and a `serverInfo.name`. 2. The host config's plugin marker on stderr: the config was composed, not the artifact beside it. Before the boot it asserts the positive control: `dist/objectstack.json` exists. The artifact is what makes the defect silent. Without one, a swallowed config path is a loud "Nothing to serve" exit. The budget line is 15 s: 11.6 s, the slowest of four local runs on a shared container, x 1.25. There is no CI reading yet. The pin spawns `bin/run.js`, so `scripts/check-cli-test-child-env.mjs`'s pinned built-entrypoint population gains it, going from eleven to twelve files, with the `Joined the population deliberately` note every member carries. **Reverse verification.** The fix was committed first (`49bd321ea`). Both legs went through `scripts/ablation-replace.mjs` (wrapped restore) and `scripts/ablation-dist-preflight.mjs`, with the CLI rebuilt for each leg. - **Mutate** (`, ignoreStdin: true` deleted): the marker is absent from all 652 built files, and the pin is **RED**: `initialize was never answered` / `no answer within 60000 ms`, with `No objectstack.config.ts found — booting from artifact (default host)...` in the transcript. - **Restore:** blob `cd1aa3d29` equals HEAD and `git diff HEAD` is empty. The rebuilt `dist/` carries the marker again, the tree is clean, and the pin then passed three times. Predicted direction: red. Observed: red. ## Local verification (head `749f1a281`, after merging `origin/main` `e46ce7b6c`) - A full `turbo run build` (76/76 tasks) ran first, because the merge moved `packages/spec`. - `pnpm --filter @objectstack/cli typecheck`: exit 0. `tsc -p tsconfig.test.json --noEmit --listFiles` lists both changed test files. - `pnpm --filter @objectstack/cli exec vitest run --project unit --maxWorkers=2`: 310 files, 4599 tests passed. - `vitest run --project integration` on the new pin and `start-mcp-stdio-early-initialize.integration.test.ts`: 2 passed. The `os start` pin was run because its `serve` child is the command changed here. The rest of the `integration` tier is declared to CI. - `OS_TEST_TIERS=nightly vitest run --project integration test/serve-mcp-stdio-answers.e2e.test.ts`: 1 passed. - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 87 commands, and all 87 exited 0 (each exit captured before any pipe). `--ran` reconciled: `87 derived, 87 run, 0 NOT-MEASURED, 0 UNRUN`. This includes `check:cli-test-child-env` (15 built-CLI spawns, all out of `development`/`test`), `check:pm-dispatch-gates` (1884 cases), `check:nul-bytes`, `check:dual-build-cjs-loads` and `check:i18n-coverage`. - Edited gate script: `node scripts/check-cli-test-child-env.mjs --self-test` passed 152 cases. No test suite exercises the script; its only other mention is prose in `serve-node-env-production-default.e2e.test.ts`. - `pnpm exec eslint --no-inline-config` over the four changed source files: exit 0, no output. The repo-wide `pnpm lint` is left to CI. ## Acceptance notes - The comment beside the `resume()` in `packages/mcp/src/mcp-server-runtime.ts` still says that `os serve --dev` with no path leaves stdin paused under oclif. After this PR that is history, not the current mechanism. The comment's general reason still holds: any host that touched stdin before `start()`. `packages/mcp` is outside this card's surface, so it is noted here and not edited. - `os compile`'s optional `config` positional (no default, no `ignoreStdin`) still reads a pending stdin line. `compile` is not an MCP door, and `os start`'s compile spawn already gives it no stdin. Noted only. - `os dev` is unchanged and is not a stdio door (see PR #24184's notes). - The e2e's preload stands in for a host that pauses stdin; it is not a production caller. After this PR no production door pauses stdin before the transport attaches. Measured: the e2e as it stood on `f963d8e2b` passes with `resume()` removed. --- _Generated by [Claude Code](https://claude.ai/code/session_01QyoWKK9RwUNxBxmevy9Neb)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent a19b2a7 commit b7f29dd

5 files changed

Lines changed: 449 additions & 9 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/cli': patch
3+
---
4+
5+
Clause-②: no
6+
7+
fix(cli): `os serve` with the stdio MCP transport and no config argument answers the `initialize` an MCP client writes when it spawns the server, and boots the configured app
8+
9+
- **What was wrong.** An MCP client writes `initialize` to the server's stdin as soon as it spawns it. The CLI's argument parser fills an omitted positional argument from a pending stdin line, and `os serve`'s config path is an optional positional. So, started without a config path, `os serve` took the client's `initialize` as its config path. In a project with a compiled `dist/objectstack.json`, it printed `No objectstack.config.ts found — booting from artifact`, booted without the config's own plugins, and never answered `initialize`. `os start` already named the path when it started `os serve`; this is the same defect for `os serve` started directly.
10+
- **What changed.** `os serve` no longer reads its config path from stdin. Without the argument it uses `objectstack.config.ts` in the working directory, as its help says, and the client's first message stays in the pipe for the stdio transport. Reading the config path from stdin was never documented, and no script, test or doc in this repository relied on it; to serve another config, pass its path as the argument.
11+
- **Unchanged:** the flags and environment variables of `os serve`, and which config and artifact a boot uses when the path is given or when nothing waits on stdin.

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1397,7 +1397,13 @@ export default class Serve extends Command {
13971397
static override description = 'Start ObjectStack server. Reads `objectstack.config.ts` if present; otherwise falls back to `dist/objectstack.json` (or OS_ARTIFACT_PATH, including http(s):// URLs) as a portable artifact.';
13981398

13991399
static override args = {
1400-
config: Args.string({ description: 'Configuration file path', required: false, default: 'objectstack.config.ts' }),
1400+
// [#24191] `ignoreStdin`: oclif otherwise fills an omitted positional from
1401+
// a line already waiting on stdin (`tryStdin`), and with the stdio MCP
1402+
// transport that line is the client's `initialize`, written at spawn. It
1403+
// became the config path, the boot fell back to the artifact without the
1404+
// config, and the request was never answered. Stdin belongs to the
1405+
// transport; the config path comes from argv or the default.
1406+
config: Args.string({ description: 'Configuration file path', required: false, default: 'objectstack.config.ts', ignoreStdin: true }),
14011407
};
14021408

14031409
static override flags = {

‎packages/cli/test/serve-mcp-stdio-answers.e2e.test.ts‎

Lines changed: 65 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,21 @@
1919
* `readableFlowing` is `null` — never after an explicit `pause()`. Listener
2020
* attached, `bytesRead` 0, server deaf.
2121
*
22+
* WHY THIS FILE NOW PAUSES STDIN ITSELF (#24191). `serve` no longer supplies
23+
* that pause: its `config` positional sets `ignoreStdin`, because oclif's
24+
* stdin read also SWALLOWED an `initialize` the client wrote at spawn (pinned in
25+
* `serve-mcp-stdio-early-initialize.integration.test.ts`). The `resume()` in
26+
* `MCPServerRuntime.start()` stays, for the reason its own comment gives: any
27+
* host that touched stdin before `start()` — a readline prompt, a supervisor,
28+
* an embedding process — leaves it paused. Without a pauser this file stayed
29+
* green with that `resume()` removed (measured on the #24191 change), so the
30+
* stdio boot now runs with a preload that IS such a host
31+
* (`PAUSE_STDIN_FIRST`): it pauses `process.stdin` before the CLI loads, and
32+
* reports `readableFlowing` at the moment the transport attaches its `data`
33+
* listener. The test asserts that report reads `false`, so the premise is
34+
* measured on every run rather than assumed, and a later change that resumes
35+
* stdin earlier cannot quietly make this file vacuous again.
36+
*
2237
* WHY THIS FILE SPAWNS THE CLI. `packages/mcp`'s 17 existing stdio pins were
2338
* all green while every real `os serve` stdio session was unusable, because
2439
* they sit BELOW the gap: they exercise the runtime in a plain node process,
@@ -30,7 +45,8 @@
3045
* the mechanism, not the consequence. The assertion below is the card's own
3146
* repro: write a real `initialize` to the child's stdin, get a real result
3247
* back. Reverse-verified — with the `resume()` in `MCPServerRuntime.start()`
33-
* removed, this file times out with zero bytes on stdout.
48+
* removed, this file times out with zero bytes on stdout (re-measured with the
49+
* preload above when #24191 moved the pause out of `serve`).
3450
*
3551
* WHY IT MINTS A KEY OVER HTTP FIRST. stdio auto-start is fail-closed
3652
* (ADR-0101): without an `OS_MCP_STDIO_API_KEY` that resolves to a real
@@ -93,6 +109,30 @@ const HERE = resolve(fileURLToPath(import.meta.url), '..');
93109
*/
94110
const CLI = resolve(HERE, '../bin/run.js');
95111

112+
/** The line the preload prints each time a `data` listener attaches to stdin. */
113+
const PREMISE_TAG = '[fx-7645] stdin data listener attached, readableFlowing=';
114+
115+
/**
116+
* Node arguments for the stdio boot: a host that paused stdin before the CLI
117+
* loaded — #7645's premise, which `serve` itself no longer supplies (#24191).
118+
*
119+
* `process.stdin.pause()` is what readline's `Interface.close()` did when
120+
* oclif's `tryStdin` paused it. The `newListener` report fires BEFORE the
121+
* listener is added, so it reads the state the transport's `data` listener
122+
* meets: `false` is the premise holding, `null` is a stream nobody paused.
123+
* A `data:` URL keeps the whole host on this page; `--import` runs it before
124+
* `bin/run.js`.
125+
*/
126+
const PAUSE_STDIN_FIRST: readonly string[] = [
127+
'--import',
128+
`data:text/javascript,${encodeURIComponent([
129+
'process.stdin.pause();',
130+
"process.stdin.on('newListener', (event) => {",
131+
` if (event === 'data') process.stderr.write(${JSON.stringify(PREMISE_TAG)} + String(process.stdin.readableFlowing) + '\\n');`,
132+
'});',
133+
].join('\n'))}`,
134+
];
135+
96136
const CONFIG = `
97137
export default {
98138
manifest: {
@@ -134,14 +174,19 @@ interface Booted {
134174
* stdout alone would wait on a stream that carries nothing until a client
135175
* speaks.
136176
*
137-
* NOTE: no positional config path is passed. That is deliberate and load-bearing:
138-
* supplying one makes oclif skip `tryStdin` entirely, so stdin is never paused
139-
* and the defect cannot reproduce. `os serve --dev` is also the form every user
140-
* types.
177+
* NOTE: no positional config path is passed. That used to be what paused stdin
178+
* (oclif's `tryStdin` ran only for an omitted positional); since #24191 `serve`
179+
* never reads stdin for it either way, and the pause comes from `nodeArgs`
180+
* (`PAUSE_STDIN_FIRST`). The bare form stays because `os serve --dev` is the
181+
* form every user types.
141182
*/
142-
function boot(env: Record<string, string | undefined>, waitFor: RegExp): Promise<Booted> {
183+
function boot(
184+
env: Record<string, string | undefined>,
185+
waitFor: RegExp,
186+
nodeArgs: readonly string[] = [],
187+
): Promise<Booted> {
143188
return new Promise((resolveBoot, rejectBoot) => {
144-
const child = spawn(process.execPath, [CLI, 'serve', '-p', port, '--dev'], {
189+
const child = spawn(process.execPath, [...nodeArgs, CLI, 'serve', '-p', port, '--dev'], {
145190
cwd: dir,
146191
stdio: ['pipe', 'pipe', 'pipe'],
147192
// `childEnv`, not a bare `...process.env`: the vitest worker exports
@@ -423,8 +468,21 @@ describe('#7645: the stdio MCP transport answers over a spawned CLI process', ()
423468
OS_MCP_STDIO_API_KEY: apiKey,
424469
},
425470
/\[MCP\] Server started \(transport: stdio/,
471+
PAUSE_STDIN_FIRST,
426472
);
427473

474+
// The premise, measured: the transport attached its reader to a stdin that
475+
// was already paused, so only `resume()` can make it flow. Every attach the
476+
// preload saw reads `false`; none at all means the preload never ran.
477+
const attaches = booted.stderr()
478+
.split('\n')
479+
.filter((line) => line.includes(PREMISE_TAG))
480+
.map((line) => line.slice(line.indexOf(PREMISE_TAG) + PREMISE_TAG.length).trim());
481+
expect(
482+
attaches.length > 0 && attaches.every((state) => state === 'false'),
483+
`stdin was not paused when the transport attached (readableFlowing at each data-listener attach: ${JSON.stringify(attaches)}), so this run never exercised #7645's resume()\n--- stderr ---\n${booted.stderr().slice(-4000)}`,
484+
).toBe(true);
485+
428486
// The started transport is the premise, not the assertion — the whole
429487
// defect was a transport that reached exactly this line and then went deaf.
430488
const reply = await new Promise<string | null>((resolveReply) => {

0 commit comments

Comments
 (0)