Signal Terminal end-of-input at stdin EOF instead of hanging - #6676
Conversation
🦋 Changeset detectedLatest commit: d0d11de The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughNodeTerminal tracks stdin EOF, closes its input queue when stdin ends, and causes ChangesNode terminal EOF handling
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
Comment |
Address review polish on the stdin EOF fix: - Reduce the changeset to a single-line summary instead of a paragraph restating the implementation. - Drop the `=== true` coercion on `stdin.readableEnded`; both uses are boolean contexts, so it never changed the result. - Collapse the `readInput` end-of-input comment to the reason it exists (consumers would hang) rather than narrating what the handler does. Comments explaining the Bun `readableEnded` gap and why `readLine` listens to readline "close" instead of stdin "end" are kept, since neither is evident from the code.
Bundle Size Analysis
|
Closes #4895
Problem
NodeTerminalnever observes stdin end-of-input:readInputends itsQueue<UserInput, Cause.Done>only whenshouldQuitmatches a Ctrl+C/Ctrl+D keypress. With piped or closed stdin (echo y | my-cli login), any prompt after the buffered keypresses are consumed blocks onQueue.takeforever — even thoughPrompt.runalready maps queue end toQuitErrorand documents that it fails "if terminal input ends".readLineonly listens for the readline"line"event, so it hangs the same way at EOF (the original report in Terminal.readLine hangs when input stream is closed #4895).Fix
maketracks end-of-input once per terminal (stdin"end"fires once per process, and Bun never setsreadableEnded, so readers created after the event cannot re-detect it from the stream).readInputends its queue on stdin"end"(or immediately when input already ended). Keypresses buffered before the end still deliver first;Prompt.runthen fails withTerminal.QuitErrorthrough the existingCause.Donemapping.readLinefails withTerminal.QuitErrorwhen the readline interface closes. Listening to readline"close"rather than stdin"end"preserves the existing behavior where a final line without a trailing newline still flushes through"line"first.Interactive TTY sessions are unaffected: a TTY stdin does not emit
"end"during normal use, and Ctrl+C/Ctrl+D in raw mode remain keypresses handled byshouldQuit.BunTerminalreuses this implementation, so both runtimes are covered.Verification
Verified end-to-end in a downstream CLI (Bun runtime) that previously shipped a wrapper Terminal layer implementing exactly this queue-end behavior: with this change applied to the installed package and the wrapper removed, its full CLI test suite passes, including
printf 'y\n' | cliprompt flows and prompt interruption via closed stdin.Happy to add a spawned-fixture test for the piped-stdin case if you'd like one — I didn't see existing NodeTerminal coverage to extend.
Summary by CodeRabbit
Terminal.QuitErrorinstead of hanging after input is closed or piped.readLinenow fails immediately for readers created after input has already ended.