diff --git a/.changeset/eighty-donuts-tickle.md b/.changeset/eighty-donuts-tickle.md new file mode 100644 index 0000000000..02ca58c84c --- /dev/null +++ b/.changeset/eighty-donuts-tickle.md @@ -0,0 +1,34 @@ +--- +'@objectstack/cli': patch +--- + +fix(cli): `os login --json` is a parseable NDJSON stream (#6531) + +`os login --json` produced output that no consumer could read in any shape. The +device flow wrote its RFC 8628 device-authorization payload compact and, once +the token poll resolved, the result payload 2-space indented — two JSON +documents on one stdout. Driven against a live device endpoint, that stream +failed `JSON.parse()` with `Unexpected non-whitespace character +after JSON at position 200`, and read as NDJSON it failed on 5 of its 6 lines, +because the second document spanned five of them. The same two-document shape +appeared on the failure path, where an error payload could follow a +device-authorization record that had already been written. + +`os login --json` is now a **newline-delimited JSON stream**: one compact +document per line, on every path — the device-authorization record, the +`--email`/`--password` result, the already-logged-in notice, and the +`{"success":false,"error":"…"}` failure record alike. Every line parses on its +own, and the verification-URL record still arrives *before* the user +authorizes, which is what makes the device flow usable from a script at all. + +This is the CLI's **one declared exception** to "`--json` means exactly one JSON +document on stdout" (#6217), and it is declared rather than silent: the +`--json` flag's `--help` text says so, and so do the CLI reference page and the +device-flow section of the authentication docs. Parse this command's stdout +line by line. + +Bumped as a patch: no interface is added or removed and nothing that previously +worked stops working. The device-flow output was unparseable before, so it had +no consumers to break; the only other observable change is that the +email/password result is compact rather than indented, which `JSON.parse` reads +identically. Human-mode output is untouched. diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index 9100dc0ce6..09437c366a 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -1078,6 +1078,40 @@ For CI and other non-interactive contexts, pass email/password directly: os login --email user@example.com --password secret ``` +##### `os login --json` is NDJSON — the one exception + +Every other ObjectStack command writes **exactly one JSON document** to stdout +under `--json`, so `JSON.parse()` is the way to read it. +`os login` is the single declared exception: its `--json` output is **NDJSON**, +one compact JSON document per line. **Parse it line by line.** + +The reason is the device flow: it is two events at two points in time, and the +verification URL is only useful to a script *before* the user authorizes. So the +CLI emits it as its own record immediately, then a second record when the poll +resolves: + +```console +$ os login --json --no-browser +{"device_code":"…","user_code":"WXYZ-1234","verification_uri":"https://…/activate","verification_uri_complete":"https://…/activate?user_code=WXYZ-1234","expires_in":600} +{"success":true,"email":"user@example.com","userId":"usr_01H…"} +``` + +Read the first record, show the user the URL, then block on the next line: + +```bash +os login --json --no-browser | while IFS= read -r line; do + echo "$line" | jq -r 'if .verification_uri_complete then "Approve at: \(.verification_uri_complete)" else "Signed in as \(.email)" end' +done +``` + +Every record is one line, on every path — the `--email`/`--password` result and +the failure payload (`{"success":false,"error":"…"}`) included, since a failure +can arrive *after* the verification-URL record has already been written. Records +that report failure also set exit code `1`. + +Before this was declared, `os login --json` wrote a compact record followed by a +pretty-printed one, which parsed as neither a single document nor as NDJSON. + #### `os logout` Logout calls `POST /api/v1/auth/sign-out` before deleting local credentials, so diff --git a/content/docs/permissions/authentication.mdx b/content/docs/permissions/authentication.mdx index 87eb877e0c..4d02d2afb5 100644 --- a/content/docs/permissions/authentication.mdx +++ b/content/docs/permissions/authentication.mdx @@ -68,6 +68,12 @@ expire after the server-configured TTL (the CLI assumes a 10-minute / 600s default). The device flow requires `plugins: { deviceAuthorization: true }` in your `AuthPlugin` configuration. +Under `--json` this command is the CLI's **one declared NDJSON exception**: it +emits the verification-URL record before you authorize and the result record +afterwards, one compact JSON document per line, so stdout must be parsed line by +line rather than with a single `JSON.parse`. See +[the CLI reference](/docs/deployment/cli#os-login--json-is-ndjson--the-one-exception). + The email/password path is still supported for CI and non-interactive shells: ```bash diff --git a/packages/cli/src/commands/login.ts b/packages/cli/src/commands/login.ts index d9c4b5e335..3145e4532c 100644 --- a/packages/cli/src/commands/login.ts +++ b/packages/cli/src/commands/login.ts @@ -1,12 +1,82 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. +/** + * `os login --json` is NDJSON — the CLI's ONE declared exception (#6531). + * + * ## What was broken + * + * Everywhere else in this CLI `--json` means "stdout is exactly one JSON + * document" (#6217). The device-flow path could not honour that and did not + * try: it wrote the RFC 8628 device-authorization payload compact, and then, + * after the token poll succeeded, the result payload 2-space indented. Measured + * against a live device endpoint, stdout came out as + * + * ``` + * {"device_code":"…","user_code":"…","verification_uri":"…","expires_in":600} + * { + * "success": true, + * … + * } + * ``` + * + * — which `JSON.parse` rejects (`Unexpected non-whitespace character after JSON + * at position 200`) *and* which is not NDJSON either, because the second + * document spans five lines: 5 of its 6 lines fail an independent parse. A + * consumer had no shape to read it in at all. The same two-document stream + * appeared on the failure path too — device record, then an indented error + * payload when the poll timed out or was denied. + * + * ## Why a stream rather than one document + * + * Maintainer ruling, 2026-08-08 (#6531): this flow genuinely IS two events at + * two points in time, and emitting the verification URL **before** the user + * authorizes is the entire value of device flow in automation. Buffering both + * halves into one trailing document would make stdout parseable by destroying + * the thing the output exists for; putting the early record on stderr would + * abuse the diagnostic stream for non-diagnostic content. So `os login --json` + * is declared a newline-delimited stream, and — the ruling's binding condition + * — declared *explicitly*: in this command's `--help` text and in the command + * documentation (`content/docs/deployment/cli.mdx`, and the device-flow section + * of `content/docs/permissions/authentication.mdx`). An undocumented exception + * does the same harm to a consumer as the bug it replaces. + * + * ## Why EVERY write, not just the device flow's two + * + * The contract belongs to the command, not to one of its paths. If the + * `--email`/`--password` result or the error payload stayed indented, a + * consumer that read this command line-by-line — exactly what the docs now + * tell it to do — would break on the first run that took another path, and the + * failure path is reachable *after* the device record has already been written. + * So every `--json` write goes through {@link emitRecord}, which is the only + * emitter in this file; that makes "one compact document per line" a property + * of the command instead of four call sites that each have to remember an + * option. `packages/cli/test/login-json-ndjson.e2e.test.ts` holds both halves: + * the stream contract, driven through a real child process against a real + * device endpoint, and the source pin that keeps a future write from bypassing + * the helper. + */ + import { Command, Flags } from '@oclif/core'; +import type { CliExitCode } from '../utils/format.js'; import { printHeader, printSuccess, printError, printKV, emitJson } from '../utils/format.js'; import { writeAuthConfig, readAuthConfig } from '../utils/auth-config.js'; import { ObjectStackClient } from '@objectstack/client'; import * as readline from 'node:readline/promises'; import { stdin as input, stdout as output } from 'node:process'; +/** + * Emit ONE NDJSON record on stdout — the only `--json` writer in this command. + * + * Compact is not a formatting preference here, it is the contract: a record + * that wrapped onto a second line would silently break every consumer reading + * this command's stdout a line at a time. Routing all four call sites through + * one helper is what makes that structural — see the file header for why the + * whole command, and not only the device flow's two writes, has to hold it. + */ +async function emitRecord(payload: unknown, exitCode: CliExitCode = 0): Promise { + await emitJson(payload, exitCode, { compact: true }); +} + /** * Prompt for a password with masked input (shows * per character). * Falls back to plain readline.question() in non-TTY environments. @@ -108,7 +178,8 @@ export default class AuthLogin extends Command { default: false, }), json: Flags.boolean({ - description: 'Output as JSON', + description: + 'Machine-readable output as NDJSON — one compact JSON document per line. Unlike every other ObjectStack command, whose --json stdout is a single document, this one is a stream: the device flow reports the verification URL as its own record BEFORE you authorize, then the result as a second record. Parse stdout line by line.', }), }; @@ -122,7 +193,7 @@ export default class AuthLogin extends Command { const existing = await readAuthConfig(); if (existing?.token) { if (flags.json) { - await emitJson({ success: false, error: 'Already logged in', email: existing.email }, 0, { compact: true }); + await emitRecord({ success: false, error: 'Already logged in', email: existing.email }); } else { printSuccess(`Already logged in as ${existing.email || existing.userId}`); console.log(''); @@ -171,7 +242,11 @@ export default class AuthLogin extends Command { await this.loginWithPassword(client, flags.url, email, password, flags.json); } catch (error: any) { if (flags.json) { - await emitJson({ success: false, error: error.message }); + // Reachable AFTER the device-authorization record has already been + // written (an expired code, a denied approval, a poll failure), so an + // indented payload here recreated the exact two-document stream #6531 + // is about — on the path a consumer is least able to recover from. + await emitRecord({ success: false, error: error.message }); this.exit(1); } printError(error.message || String(error)); @@ -206,7 +281,7 @@ export default class AuthLogin extends Command { }); if (jsonOutput) { - await emitJson({ success: true, email: user?.email || email, userId: user?.id }); + await emitRecord({ success: true, email: user?.email || email, userId: user?.id }); } else { printSuccess('Authentication successful'); printKV('Email', user?.email || email); @@ -252,7 +327,10 @@ export default class AuthLogin extends Command { const verificationUrl = verification_uri_complete || `${verification_uri}?user_code=${encodeURIComponent(user_code)}`; if (jsonOutput) { - await emitJson({ device_code, user_code, verification_uri, verification_uri_complete, expires_in }, 0, { compact: true }); + // Record 1 of 2, and deliberately written BEFORE the poll loop: an + // automation consumer needs the verification URL while it can still act + // on it, which is the reason this command is a stream at all. + await emitRecord({ device_code, user_code, verification_uri, verification_uri_complete, expires_in }); } else { console.log(' To authorize this CLI, visit:'); console.log(''); @@ -318,7 +396,8 @@ export default class AuthLogin extends Command { }); if (jsonOutput) { - await emitJson({ success: true, email: user?.email, userId: user?.id }); + // Record 2 of 2 — same line-per-document shape as record 1. + await emitRecord({ success: true, email: user?.email, userId: user?.id }); } else { printSuccess('Authentication successful'); if (user?.email) printKV('Email', user.email); diff --git a/packages/cli/src/utils/format.ts b/packages/cli/src/utils/format.ts index 9e50466a01..a446826a6d 100644 --- a/packages/cli/src/utils/format.ts +++ b/packages/cli/src/utils/format.ts @@ -47,11 +47,22 @@ export interface EmitJsonOptions { * Exists so the sweep onto `emitJson` could be a pure truncation fix with no * observable output change: roughly half the CLI's `--json` sites were * already compact and half indented, and this preserves whichever each one - * emitted. The split is accidental rather than designed — `os login --json` - * prints a compact payload and then an indented one in the same run — so - * unifying it is worth doing, but as its own decision, not as a side effect - * of fixing truncated pipes. + * emitted. * + * This comment used to cite `os login --json` — a compact payload followed by + * an indented one in the same run — as proof the split was accidental. That + * was true, and worse than a formatting inconsistency: two documents on one + * stdout parse as neither a single document nor as NDJSON. #6531 fixed it, + * and in doing so gave `compact` its one *designed* use. `os login` is the + * CLI's sole declared NDJSON command, because its device flow is genuinely + * two events over time and the first one has to reach an automation consumer + * before the user authorizes; there, one line per document IS the contract, + * enforced through a single emitter in `commands/login.ts` and pinned by + * `test/login-json-ndjson.e2e.test.ts`. + * + * Everywhere else `--json` still means exactly one JSON document on stdout + * (#6217), so the remaining compact call sites are still only preserving + * historical formatting and unifying them stays worth doing on its own. * New code should use the default. */ compact?: boolean; diff --git a/packages/cli/test/login-json-ndjson.e2e.test.ts b/packages/cli/test/login-json-ndjson.e2e.test.ts new file mode 100644 index 0000000000..c232a7ab62 --- /dev/null +++ b/packages/cli/test/login-json-ndjson.e2e.test.ts @@ -0,0 +1,381 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os login --json` is NDJSON — every line parses, and the URL comes FIRST + * (#6531). + * + * ## The defect this pins shut + * + * The device-flow path wrote its RFC 8628 device-authorization payload compact + * and, after the token poll resolved, the result payload 2-space indented. + * Measured against a live device endpoint on `origin/main`, stdout was: + * + * ``` + * {"device_code":"DEV-CODE-6531","user_code":"WXYZ-6531",…,"expires_in":600} + * { + * "success": true, + * "email": "device@example.com", + * "userId": "usr_6531" + * } + * ``` + * + * `JSON.parse(stdout)` → `Unexpected non-whitespace character after JSON at + * position 200`; read as NDJSON, 5 of the 6 lines fail their own parse. There + * was no shape in which a consumer could read it. The maintainer ruled + * (2026-08-08) that this command is a **stream**, so what has to hold is: + * every line parses on its own, and the records arrive in the right order. + * + * ## Why the ordering assertion is TEMPORAL, not an array index + * + * The ruling picked NDJSON over "buffer both halves and emit one document at + * the end" for exactly one reason: an automation consumer needs the + * verification URL *while it can still act on it* — before the user authorizes. + * Asserting `records[0]` is the device record would not catch a regression to + * the buffered shape at all, because a trailing merged emit also puts the URL + * fields first. + * + * So the fake endpoint here withholds the token until THIS TEST has seen the + * device record land on the child's stdout. The release is driven by the + * observation, which makes "URL before authorization" a fact about the run + * rather than a property of the array afterwards. A buffered implementation + * never gets released by the watcher, falls through to the escape-hatch timer + * ({@link RELEASE_DEADLINE_MS}, so the suite fails instead of hanging), and + * lands with `urlSeenAt === null` — red, naming the actual cause. + * + * ## Why a PTY, and why not a silent skip + * + * `login.ts` takes the device flow only when `process.stdin.isTTY` — a child + * with a piped stdin falls through to the email/password prompt and never + * reaches either emission point. A plain `execFile` therefore cannot reach the + * defect. `script(1)` allocates the pty; the command it runs redirects stdout + * and stderr to files, so fd 0 is a TTY while fd 1 and fd 2 stay separate, + * capturable, and non-TTY — which is also the shape a real user gets when they + * pipe an interactive `os login --json` into a consumer. + * + * If `script(1)` is missing this file FAILS rather than skips. A contract test + * that quietly opts out on the machine that runs it is the same green-for-the- + * wrong-reason hazard the fixtures in #5046 were replaced for; every CI runner + * in this repo is `ubuntu-latest`, where `script(1)` is part of the base image. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFile, execFileSync } from 'node:child_process'; +import { createServer, type Server } from 'node:http'; +import type { AddressInfo } from 'node:net'; +import { mkdtempSync, rmSync, readFileSync, existsSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); +const LOGIN_SRC = resolve(HERE, '../src/commands/login.ts'); +const REPO_ROOT = resolve(HERE, '../../..'); +const CLI_DOCS = resolve(REPO_ROOT, 'content/docs/deployment/cli.mdx'); +const AUTH_DOCS = resolve(REPO_ROOT, 'content/docs/permissions/authentication.mdx'); + +/** + * How long the endpoint waits for the device record to appear before releasing + * anyway. Only reached when the record never arrives early — i.e. when the + * contract is broken — and exists so that failure is an assertion rather than a + * suite that hangs until the runner kills it. + */ +const RELEASE_DEADLINE_MS = 20_000; + +/** Poll interval for watching the child's stdout file. */ +const WATCH_MS = 25; + +interface DeviceRun { + code: number; + stdout: string; + stderr: string; + /** When the device-authorization record was first readable on stdout. */ + urlSeenAt: number | null; + /** When the endpoint first answered the poll with something other than pending. */ + authorizedAt: number | null; + /** Whether the endpoint had to fall back to the deadline instead of the watcher. */ + releasedByDeadline: boolean; +} + +/** + * A minimal RFC 8628 endpoint whose token poll stays `authorization_pending` + * until {@link release} is called — see the header for why the test, not the + * clock, decides when authorization happens. + */ +function startDeviceEndpoint(outcome: 'token' | 'access_denied') { + let released = false; + let authorizedAt: number | null = null; + + const server: Server = createServer((req, res) => { + req.resume(); + req.on('end', () => { + const send = (code: number, obj: unknown) => { + res.writeHead(code, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify(obj)); + }; + const { pathname } = new URL(req.url ?? '/', 'http://placeholder'); + + if (pathname === '/api/v1/auth/device/code') { + return send(200, { + device_code: 'DEV-CODE-6531', + user_code: 'WXYZ-6531', + verification_uri: 'http://127.0.0.1:9/activate', + verification_uri_complete: 'http://127.0.0.1:9/activate?user_code=WXYZ-6531', + expires_in: 600, + // 1s: the CLI floors anything falsy back to 5s (`interval || 5`). + interval: 1, + }); + } + + if (pathname === '/api/v1/auth/device/token') { + if (!released) return send(400, { error: 'authorization_pending' }); + authorizedAt ??= Date.now(); + return outcome === 'token' + ? send(200, { access_token: 'ACCESS-TOKEN-6531', token_type: 'Bearer' }) + : send(400, { error: 'access_denied' }); + } + + if (pathname === '/api/v1/auth/get-session') { + return send(200, { user: { id: 'usr_6531', email: 'device@example.com' } }); + } + + return send(404, { error: 'not_found' }); + }); + }); + + return { + server, + release: () => { released = true; }, + authorizedAt: () => authorizedAt, + listen: () => + new Promise((res) => { + server.listen(0, '127.0.0.1', () => res((server.address() as AddressInfo).port)); + }), + close: () => new Promise((res) => server.close(() => res())), + }; +} + +/** Every non-empty line of a captured stdout. */ +function lines(stdout: string): string[] { + return stdout.split('\n').filter((l) => l.length > 0); +} + +/** The device-authorization record, if one is already readable on stdout. */ +function findDeviceRecord(text: string): Record | null { + for (const line of lines(text)) { + try { + const rec = JSON.parse(line) as Record; + if (rec && typeof rec === 'object' && 'verification_uri' in rec) return rec; + } catch { + // A partially-flushed line is not a failure here — the contract + // assertions below judge the finished stream. + } + } + return null; +} + +/** + * Drive one full device-flow login through a real child process on a PTY. + */ +async function runDeviceLogin(outcome: 'token' | 'access_denied'): Promise { + const dir = mkdtempSync(join(tmpdir(), 'os-login-ndjson-')); + const outFile = join(dir, 'stdout.txt'); + const errFile = join(dir, 'stderr.txt'); + // HOME is the credentials root (`auth-config.ts` builds every path from + // `os.homedir()`), so pointing it at the temp dir keeps the run from reading + // — or overwriting — the developer's real ~/.objectstack/credentials.json. + const home = join(dir, 'home'); + + const endpoint = startDeviceEndpoint(outcome); + const port = await endpoint.listen(); + + let urlSeenAt: number | null = null; + let releasedByDeadline = false; + + const watcher = setInterval(() => { + if (urlSeenAt !== null) return; + if (!existsSync(outFile)) return; + if (findDeviceRecord(readFileSync(outFile, 'utf-8'))) { + urlSeenAt = Date.now(); + endpoint.release(); + } + }, WATCH_MS); + + const deadline = setTimeout(() => { + if (urlSeenAt === null) { + releasedByDeadline = true; + endpoint.release(); + } + }, RELEASE_DEADLINE_MS); + + const shell = [ + `'${TSX}' '${CLI}' login --json --no-browser`, + `--url 'http://127.0.0.1:${port}'`, + `> '${outFile}' 2> '${errFile}'`, + ].join(' '); + + const code = await new Promise((res) => { + execFile( + 'script', + // -q quiet, -e propagate the child's exit status, -c the command. + ['-qec', shell, '/dev/null'], + { env: { ...process.env, HOME: home, NO_COLOR: '1' }, maxBuffer: 32 * 1024 * 1024 }, + (err) => res(err ? Number((err as { code?: unknown }).code ?? 1) : 0), + ); + }); + + clearInterval(watcher); + clearTimeout(deadline); + await endpoint.close(); + + const read = (f: string) => (existsSync(f) ? readFileSync(f, 'utf-8') : ''); + const run: DeviceRun = { + code, + stdout: read(outFile), + stderr: read(errFile), + urlSeenAt, + authorizedAt: endpoint.authorizedAt(), + releasedByDeadline, + }; + rmSync(dir, { recursive: true, force: true }); + return run; +} + +describe('os login --json — the declared NDJSON stream (#6531)', () => { + let ok: DeviceRun; + let denied: DeviceRun; + + beforeAll(async () => { + try { + execFileSync('script', ['--version'], { stdio: 'ignore' }); + } catch { + throw new Error( + 'script(1) is required to drive the TTY-gated device flow — see this file’s header for why this fails instead of skipping.', + ); + } + // Sequential: each run owns a port, a HOME and a poll clock, and running + // them together would interleave two children's timing for no benefit. + ok = await runDeviceLogin('token'); + denied = await runDeviceLogin('access_denied'); + }, 180_000); + + describe('the successful flow', () => { + it('emits stdout every line of which parses on its own — the NDJSON contract', () => { + const all = lines(ok.stdout); + expect(all.length).toBeGreaterThan(0); + for (const [i, line] of all.entries()) { + // Named per line so a regression says WHICH record broke, rather than + // only that some parse failed. Under the defect, lines 2-6 were the + // fragments of one pretty-printed document. + expect(() => JSON.parse(line), `stdout line ${i + 1}: ${JSON.stringify(line)}`).not.toThrow(); + } + }); + + it('is exactly two records — device authorization, then the result', () => { + const records = lines(ok.stdout).map((l) => JSON.parse(l) as Record); + expect(records).toHaveLength(2); + expect(records[0]).toMatchObject({ + device_code: 'DEV-CODE-6531', + user_code: 'WXYZ-6531', + verification_uri: 'http://127.0.0.1:9/activate', + verification_uri_complete: 'http://127.0.0.1:9/activate?user_code=WXYZ-6531', + expires_in: 600, + }); + expect(records[1]).toEqual({ success: true, email: 'device@example.com', userId: 'usr_6531' }); + expect(ok.code).toBe(0); + }); + + it('hands over the verification URL BEFORE authorization — the reason this route was chosen', () => { + // The endpoint only authorized because the watcher had already read the + // record off stdout, so these two facts are the run's own history. + expect(ok.releasedByDeadline, 'the device record never reached stdout early').toBe(false); + expect(ok.urlSeenAt).not.toBeNull(); + expect(ok.authorizedAt).not.toBeNull(); + expect(ok.urlSeenAt!).toBeLessThanOrEqual(ok.authorizedAt!); + }); + + it('lets nothing but the payload onto stdout — no banner, no spinner, no prompt', () => { + // Two records, two lines: any human-mode output would raise the count, + // so this subsumes "nothing else was written" rather than listing + // banners that a future edit could add to. + expect(lines(ok.stdout)).toHaveLength(2); + expect(ok.stdout).not.toContain('To authorize this CLI'); + expect(ok.stdout).not.toContain('Waiting for browser approval'); + expect(ok.stdout).not.toContain('ObjectStack Login'); + // The spinner's carriage returns and the `\x1b[K` erase would corrupt a + // line-oriented reader even though they carry no visible text. + expect(ok.stdout).not.toMatch(/[\r\u001b]/); + }); + }); + + describe('the failure that arrives AFTER the first record', () => { + it('keeps every line parseable when the poll is denied', () => { + // The path the issue never named: device record already written, then an + // indented error payload — the same unreadable two-document stream, on + // the run a consumer can least afford to misread. + const all = lines(denied.stdout); + for (const [i, line] of all.entries()) { + expect(() => JSON.parse(line), `stdout line ${i + 1}: ${JSON.stringify(line)}`).not.toThrow(); + } + const records = all.map((l) => JSON.parse(l) as Record); + expect(records).toHaveLength(2); + expect(records[0]).toMatchObject({ user_code: 'WXYZ-6531' }); + expect(records[1]).toEqual({ success: false, error: 'Login denied by user.' }); + }); + + it('reports the refusal through the exit code as well as the record', () => { + expect(denied.code).toBe(1); + }); + }); +}); + +describe('the exception stays declared, not just implemented (#6531 ruling)', () => { + const loginSrc = () => readFileSync(LOGIN_SRC, 'utf-8'); + + it('routes every --json write through the single compact emitter', () => { + // The contract is "one document per line" for the WHOLE command, so a new + // write that called `emitJson` directly could reintroduce a multi-line + // record on a path the e2e above does not drive. One emitter is what makes + // that structurally impossible; this is the guard on the emitter. + const src = loginSrc(); + const direct = src + .split('\n') + .map((line, i) => ({ line, n: i + 1 })) + .filter(({ line }) => /\bemitJson\s*\(/.test(line)) + .filter(({ line }) => !/^\s*await emitJson\(payload, exitCode, \{ compact: true \}\);$/.test(line)); + expect( + direct.map(({ n, line }) => `${n}: ${line.trim()}`), + 'every --json write in login.ts must go through emitRecord()', + ).toEqual([]); + expect(/async function emitRecord\(/.test(src)).toBe(true); + }); + + it('declares NDJSON in the --json flag help text', () => { + // `--help` is where a consumer looks first, and the ruling made the + // declaration a condition of the fix: an undocumented exception harms the + // same audience as the bug. + const src = loginSrc(); + const flag = /json:\s*Flags\.boolean\(\{[\s\S]*?\}\)/.exec(src)?.[0] ?? ''; + expect(flag).toMatch(/NDJSON/); + expect(flag).toMatch(/per line/i); + }); + + // Prose in .mdx is hard-wrapped, so these patterns treat any run of + // whitespace as a word gap. A pin that broke when a sentence rewrapped would + // train the next editor to delete it rather than to keep the declaration. + it('documents the exception on the CLI reference page', () => { + const doc = readFileSync(CLI_DOCS, 'utf-8'); + expect(doc).toMatch(/`os\s+login\s+--json`\s+is\s+NDJSON/); + expect(doc).toMatch(/one\s+compact\s+JSON\s+document\s+per\s+line/i); + expect(doc).toMatch(/parse\s+it\s+line\s+by\s+line/i); + }); + + it('documents the exception where the device flow itself is described', () => { + const doc = readFileSync(AUTH_DOCS, 'utf-8'); + expect(doc).toMatch(/NDJSON/); + expect(doc).toMatch(/line\s+by\s+line/i); + }); +}); + +afterAll(() => { /* every run cleans up its own temp dir */ });