fix(cli): os login --json refuses in a non-interactive shell instead of prompting on stdout (#6728) - #6985
Draft
os-project-manager wants to merge 1 commit into
Draft
Conversation
…d of prompting on stdout (#6728) The non-TTY fallback below the device flow wrote `readline`'s prompt to the `output` stream it was built on — `process.stdout`, unconditionally, `--json` or not. Measured on origin/main @ 73bff86, `os login --json --url … < /dev/null` produced exit 13 and a stdout consisting entirely of `Email: `, with no trailing newline. `--password` alone gave the same; `--email` alone gave `Password: `. Per the maintainer ruling of 2026-08-09 (shape 1), a `--json` run that would otherwise have to prompt now emits one record through the existing `emitRecord()` NDJSON emitter and exits 1: {"success":false,"error":"email and password are required in a non-interactive shell"} Separately and in the same PR: EOF on stdin now produces a defined `CliExitCode`. Exit 13 was Node's unsettled-top-level-await teardown — `readline`'s question promise is abandoned rather than rejected at EOF, so nothing threw and the command never decided anything. Every prompt is now bound to an abort that fires on the interface's `close`, which puts the failure back on the path that ends in `this.exit(1)`. The non-`--json` path keeps its prompts and gains a named error instead of a teardown. The two non-TTY prompts share one readline interface now; `promptPassword` loses its non-TTY branch, which was the second interface the unsettleable question lived in. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01USNUyHEr7uaU6MoEWXitei
|
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:
|
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
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 #6728
What was broken
Below the device flow that #6531 fixed sat a second path with the same
stdout-purity harm and a different cause. With no TTY and one or both of
--email/--passwordmissing — what a CI runner produces when a secret failsto interpolate —
login.tsfell through toreadline, whose prompt goes tothe
outputstream the interface was built on:process.stdout,unconditionally,
--jsonor not.Reproduced on unmodified
origin/main@73bff86(tsx bin/run-dev.js, tempHOME, piped stdin — no pty needed on this path, unlike the device-flowsibling):
cat -Aprints no trailing$, i.e. no trailing newline: the stringEmail:was the entire stdout of a run under a declared machine-readable flag — not
a JSON document, not NDJSON, no payload. stderr carried only
Warning: Detected unsettled top-level await. The two partial-flagcombinations reproduce the same way:
--jsonEmail:--json --password secretEmail:--json --email a@b.cPassword:--json(human path)Email:The fourth row is the second half of the defect on its own: exit 13 is Node's
Unsettled Top-Level Await teardown, not a code this CLI chose.
CliExitCode(packages/cli/src/utils/format.ts) still admits0and1only — verified, PM assumption 2 holds.
What this changes
Per the maintainer ruling of 2026-08-09 (issue comment 5229992975) — shape 1,
refuse — plus the exit-code half the ruling attached to the same PR.
1.
--jsonrefuses instead of prompting. A--jsonrun that wouldotherwise have to ask emits one record through
emitRecord()— the file'ssingle
--jsonwriter introduced by #6727, no second write path added — andexits
1:The refusal returns rather than calling
this.exit(1)inside the command's owntry:this.exitthrows, andlogin.ts's catch does not filterisExitSignal, so an exit from inside thetrywould emit a second record —the exact double-document hazard
format.tsdocuments.emitRecord(payload, 1)sets
process.exitCodeand lets the command return cleanly.One judgement call, flagged rather than buried. The ruling's condition
reads "with no
--email/--passwordand no TTY". The check here is on theflag alone, because reaching that line under
--jsonalready means the onlyway forward was a prompt, and the ruling's contract sentence is
"
--jsonmeans non-interactive by definition" — a TTY that happens to beattached does not un-declare the run. Concretely this also covers
os login --json --email me@x.comtyped in a terminal, which used to promptfor the password on stdout. If the maintainer wants that one case to keep
prompting, the change is one added
&& !process.stdin.isTTY.2. EOF on stdin produces a defined
CliExitCode. PM assumption 3 isconfirmed: the mechanism is not an error to catch, it is a promise that never
settles.
readline'squestion()resolves only when a line arrives; at EOFthe interface emits
'close'and the promise is abandoned, so nothingthrows, the outer catch never runs, and the top-level
awaitinbin/run.jsis torn down as unsettled.
askOrFailAtEof()binds each question to anAbortControlleraborted by that'close', which rejects it and puts thefailure back on the path that ends in
this.exit(1).Without
--jsonthe prompts stay — the ruling changed the machine-readablecontract, not the human one — but the ending is now:
3. The two non-TTY prompts share one
readlineinterface.promptPasswordused to open a private one for its non-TTY branch — the second interface the
unsettleable question lived in. That branch is deleted; the function is now
TTY-only and the caller guards it, so a future prompt cannot reintroduce the
hang by opening its own interface.
Verification
Reverse verification, direction predicted before running. Predicted: the
--jsoncases go red on the payload and on the exit code, the EOF cases gored on the exit code only, and the two "must keep working" controls stay green
because the defect never broke them. Reverting
login.tstoorigin/mainwiththe new test file in place gives exactly that — 17 red, 2 green:
The exit code is asserted as a set member, not as "non-zero". A
not.toBe(0)assertion stays green against the defect itself, since 13 is asnon-zero as 1 is. Each case asserts membership in
[0, 1]and the specificvalue, and it is the membership assertion whose message names 13.
New tests —
packages/cli/test/login-json-noninteractive.e2e.test.ts, 20cases through a real child process with piped stdin:
--jsoncombinations (neither /--passwordonly /--emailonly): the parsed record equals
{success:false, error:"…"}verbatim, exitis
1and a member of the defined set, stdout is exactly one line, containsno
Email:/Password:/ banner / carriage return / escape, and ends in anewline (the defect's stdout had none);
--json: exit1and a member of the set, themessage names the cause and the remedy, and stderr no longer carries
unsettled top-level await;sign-in/emailendpoint:
--json --email --passwordyields one{success:true,…}recordand exit 0, and a consumer that answers the prompts still logs in (which is
also what proves the single shared interface hands over both answers);
rl.question(inlogin.tscarries the abortsignal,
askOrFailAtEofexists, the--jsonhelp declares the implication,and the CLI reference documents the refusal string.
Suites and gates (all under the shared verification lock):
pnpm --filter @objectstack/cli test—Test Files 101 passed (101),Tests 1064 passed (1064)pnpm --filter @objectstack/cli typecheck— passcheck:*step enumerated from.github/workflows/lint.yml, run one byone: 34/34 in the ESLint job (
lint,check:nul-bytes,check:route-envelope,check:error-code-casing,check:engine-double-contract, … ) and theTypeScript Type Check job's steps including
check:type-check-coverage,check:type-check-debt,check:i18n,check:i18n-coverage, the speccheck:*family, the fullturbo run build/typechecksweeps and the examples typecheck — all pass.check:i18n,check:i18n-coverageandcheck:type-check-debtfirstreported PREREQUISITE NOT MET and went green once the closure was built, as
lint.yml does before those steps.
Out of scope
Filed as #6984 (observation-class,
finding, unassigned):pnpm check:nul-bytesenumeratesgit ls-files, so a brand-new file is notscanned until it is staged. Hit live here — a raw
0x1bwas materialized intothe new test file while writing about control characters, the gate exited 0,
and AGENTS.md's
grep -naPself-scan is what found it. CI is unaffected (aPR's tree is fully tracked); the harm is the pre-push run being green for a
reason unrelated to the bytes. The byte is gone: the test builds its control
class from
String.fromCharCodeso the source cannot carry what it assertsagainst.
Generated by Claude Code