fix(cli): reserve stdout for the --json payload across the bootSchemaStack family (#6217) - #6524
Merged
Merged
Conversation
…maStack family (#6217) `--json` has exactly one audience — a program — and every subcommand that boots a kernel was handing that program a stream it could not parse. `ObjectLogger` routes `debug`/`info`/`warn` to stdout and only `error`/`fatal` to stderr, so `os migrate recorded-by --json` produced "~60 INFO lines + payload + 2 shutdown lines" on stdout while stderr stayed empty. The only recourse was a "find the last lone `{` and its matching `}`" extractor — #4873 was forced to write one to assert its own payload — and that heuristic silently picks the wrong text as soon as a log line looks like JSON. This takes route 1 from the issue (redirect to stderr), not route 2 (drop the kernel to `logLevel: 'silent'`): route 2 throws the operator's diagnostics away, warnings included, and could not quiet the one line that never goes through the logger at all (`console.log` in `loadArtifactBundle`: `[StandaloneStack] no compiled artifact …`). Route 3 (make the logger default to stderr in `packages/core`) would change `os serve` / `os dev` output for every existing user and is a maintainer call — untouched here. The fix lands on the family's shared boot seam: `bootSchemaStack` gains a REQUIRED `jsonOutput` option. Before the boot can print its first byte it takes over `process.stdout.write` and forwards everything the kernel and its plugins write to stderr — nothing is discarded — while the payload goes out through `writeStdoutDirect` on the real stdout. `shutdown()` gives stdout back only after the kernel is fully down, so the two shutdown lines cannot land under the payload either. A failed boot deliberately keeps the reservation: the command's next act on that path is to emit its error payload, and a half-started kernel can still log. The option is required so a family member added later has to decide at compile time instead of inheriting the bug. All nine members covered: `os migrate plan` / `apply` / `resume` / `recorded-by` / `summary-nulls` / `value-shapes` / `files-to-references`, `os migrate meta --stored`, and `os meta resync`. Human-mode output is unchanged (verified: stdout identical, stderr still empty). Tests: - `packages/cli/test/json-stdout-purity.e2e.test.ts` — the shared family expectation. Discovers the members from source (calls `bootSchemaStack` AND declares a `--json` flag), reconciles that set against the driven list, then runs each as a real child process asserting a bare `JSON.parse(stdout)`, no logger record anywhere on stdout, and the boot diagnostics still present on stderr (so a regression toward silencing goes red too). A new member that is not driven goes red. - `packages/cli/test/migrate-exit-code.e2e.test.ts` — the `jsonPayload()` heuristic extractor #4873 wrote under duress is deleted; it is now a bare `JSON.parse(stdout)`. - `packages/cli/src/utils/json-stdout.test.ts` — the mechanism at unit level. Reverse verification (predicted red, and red): with the reservation disabled, 27 of the family e2e's 28 cases fail — the one that stays green is the member-inventory case, which asserts a set rather than behaviour — and the exit-code pin fails with `SyntaxError: Unexpected token 'S', "[Standalone"...`, exactly the error recorded in the issue. Fixes #6217 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017uFVNMmTxLpmfQYiuKM1Yx
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 21 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
github-actions
Bot
added
documentation
os-project-manager
marked this pull request as ready for review
August 8, 2026 04:39
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6217
The defect
--jsonhas exactly one audience — a program — and every subcommand that boots a kernel handed that program a stream it could not parse.ObjectLoggerroutesdebug/info/warnto stdout and onlyerror/fatalto stderr (packages/core/src/logger.ts:343), soos migrate recorded-by --jsonproduced roughly 60 INFO lines above the payload and two shutdown lines below it, with stderr completely empty:The consumer's only recourse was a "find the last lone
{and its matching}" extractor. #4873 was forced to write one to assert its own payload (packages/cli/test/migrate-exit-code.e2e.test.ts'sjsonPayload()), and that heuristic silently picks the wrong text as soon as a log line looks like JSON.Premise verified against
origin/mainbefore implementing — reproduced exactly as filed:Which route, and why
The issue left three routes open. Taken: route 1 — redirect to stderr.
logLevel: 'silent'under--json) was rejected on two counts. It throws the operator's diagnostics away, warnings included; and it cannot quiet the one line that never passes through the kernel logger at all —loadArtifactBundle's bareconsole.log, which prints[StandaloneStack] no compiled artifact …straight to stdout. Route 2 alone would leave the invariant broken on any uncompiled project.packages/core) is out of scope by ruling and stays untouched.A correction to the dispatch note worth recording:
packages/cli/src/commands/plugin/build.ts:180andpackages/cli/src/utils/build-runtime.ts:143were cited as evidence that route 2 is reachable today. Both are esbuild'slogLevel: 'silent', not the kernel logger's — they are options passed toesbuild.build({ … }). So they say nothing about the kernel logger either way. Reachability was established independently instead:LoggerConfig(packages/spec/src/system/logging.zod.ts) carrieslevel,format,fileandrotationbut no destination or stream knob, so a destination change cannot be expressed through config at all — which is why the redirection happens on the stream itself.os servealready does exactly this to keep its startup banner readable: it swapsprocess.stdout.writefor the boot window and buffers what it intercepts (packages/cli/src/utils/boot-log-capture.ts, #4012). This is that same, already-proven CLI-layer seam, pointed at stderr instead of at a buffer. Nopackages/corechange was needed.The fix
packages/cli/src/utils/json-stdout.ts(new) owns the reservation, and the shared boot seam installs it.bootSchemaStackgains a requiredjsonOutput: boolean. It is taken before the boot can print its first byte —createStandaloneStackannounces a missing artifact before any plugin is constructed, so a reservation one statement later already arrives too late.process.stdout.writeforwards toprocess.stderr.write, varargs and drain callback intact. Measured: this catches direct writes andconsole.log/console.info/console.debug, because Node resolvesprocess.stdoutand calls.writeon it per record — andObjectLogger.write()does the same.writeStdoutDirect, which holds the real write;emitText(and thereforeemitJson) is built on it. That is the only thing that may reach stdout during a reservation, which is what makes "exactly one JSON document" structural.shutdown()releases stdout after the kernel is fully down —kernel.shutdown()is itself two INFO lines, and those printed below the payload.Required rather than optional so a member added to this family later has to decide at compile time instead of inheriting the bug. Booting the shared stack is what makes a command a member, so this is the one call it cannot avoid.
Family verified against
origin/main(the issue's list still holds; all nine declare ajsonflag and callbootSchemaStack):os migrate plan/apply/resume/recorded-by/summary-nulls/value-shapes/files-to-references,os migrate meta --stored,os meta resync. One clarification:os migrate metaboots only under--stored, which is why the bare form was never affected andmigrate-meta.e2e.test.tscould alwaysJSON.parse(stdout).Human-mode runs are unchanged — verified on the real CLI: stdout byte-identical in shape, stderr still empty.
Tests
packages/cli/test/json-stdout-purity.e2e.test.ts(new) — the shared family expectation. The contract has one implementation face per command, so the family is discovered from source (callsbootSchemaStackAND declares a--jsonflag) and reconciled against the driven list. Each member then runs as a real child process, asserting (a) a bareJSON.parse(stdout)succeeds — no extraction, (b) no kernel-logger record and no[StandaloneStack]anywhere on stdout, (c) the boot diagnostics are still present on stderr. (c) is what makes a regression toward route 2 go red here too: a fix that deletes the operator's diagnostics is a different defect. A newly added family member goes red until it is driven.[StandaloneStack] no compiled artifact …— theconsole.logthat never touches the logger — stays in play as the second, independent pollution source.packages/cli/test/migrate-exit-code.e2e.test.ts— thejsonPayload()heuristic extractor is deleted; it is now a bareJSON.parse(stdout). TheGraceful shutdown completereceipt moved to stderr and is asserted there, with a matching negative assertion on stdout.packages/cli/src/utils/json-stdout.test.ts(new) — the mechanism at unit level: forwarding, the drain callback, the payload's way out, release, nested reservation. Note the globalconsoleis not usable as evidence inside a vitest worker (vitest replaces it), so the console coverage there goes through anode:consoleConsolebound to the process streams, and the real global-console proof is the e2e's[StandaloneStack]assertion.Reverse verification — predicted red, and red
With the reservation disabled (
bootSchemaStackignoringjsonOutput, everything else untouched):27 of 28 fail. The one that stays green is the member-inventory case, which asserts a set rather than behaviour — expected, and the reason it is a separate case.
That
SyntaxErroris verbatim the error recorded in the issue — the removal ofjsonPayload()is what exposes it.Green runs
Every gate enumerated from
.github/workflows/lint.ymlwas run individually —pnpm lintplus all 29check:*steps of the ESLint job, and every step of the TypeScript Type Check job (workspace build + typecheck, the spec generated-artifact gates, i18n, ratchets). All pass.Generated by Claude Code