Repository navigation
Conversation
A web login started a thread that read stdin until a line end, and nothing stopped that thread when the login ended. On a terminal it took the first line typed at a postpublish question, or at the OTP question of a later package. From a piped stdin it took blocks of 1024 bytes. The wait between two requests to the done url is now the wait for ENTER, so no reader of stdin outlives the login. A terminal is read through a second, non-blocking open of it. A stdin that is not a terminal is read one byte at a time, up to the first line end. Windows makes no offer of ENTER, because its opener cannot be spawned.
|
Updated 8:38 AM PT - Oct 25th, 2026
❌ @robobun, your commit b46ac22 has 1 failures in
The baseline build crashed or failed tests under 🧪 To try this PR locally: bunx bun-pr 46884That installs a local version of the PR into your bun-46884 --bun |
|
Status: ready for review. The review findings are answered in their threads, and b46ac22 has the changes. How it was reproduced
PR: #46884 |
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Input offer and prompt handling src/runtime/cli/publish_command.rs, test/cli/install/bun-publish.test.ts |
Unix input polling offers ENTER-to-open behavior when the auth URL is safe to display; non-Unix platforms do not offer it. Tests cover input during and after login, later prompts, output redirection, and recursive publishing. |
Retry timing and login completion src/runtime/cli/publish_command.rs, test/cli/install/bun-publish.test.ts |
HTTP 202 responses use a retry deadline and wait only for the remaining interval. Tests cover ENTER during a retry wait and verify supplied or default retry timing. |
Suggested reviewers: jarred-sumner
Priority:
Merge Risk: 
Low · up to b46ac
-
Publishing remains mergeable, but a fragmented piped answer can be truncated before it reaches a later prompt.
Pre-merge checks: 
4
-
Full details: Description check
-
Explanation
The description clearly explains the problem, implementation, trade-offs, platform behavior, and verification results. It does not use the template headings exactly, but it provides the required information in equivalent sections and is substantially complete.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
The deadline of the retry wait stopped the read of a piped stdin after one byte when the wait was zero, so a line was taken in parts and the next reader got the rest of it. The deadline is now only for input that is not there yet. A wait ends the offer after 1024 bytes without a line end, so a stdin such as /dev/zero cannot hold up the login. The line that is typed during the last request is taken on every terminal stdin, and not only on one that could be opened a second time. When the thread for the opener cannot start, that is reported.
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Re-checked after b46ac22: the second commit does settle the earlier points on the one-byte read under retry-after: 0, the dropped thread-spawn error, the /dev/zero spin (now bounded at 1024 bytes), and the late ENTER on a terminal whose reopen failed. The gate at EnterOffer::new (:152) and the blocking read of fd 0 in the Stdin arm (:194) are unchanged from the first push, so those threads stay open as they are; beyond the two new inline nits I have nothing further to add, and a human look at the tty/poll handling across macOS is still worthwhile.
Extended reasoning...
The PR reworks the "press ENTER to open in browser" offer of bun publish web logins in src/runtime/cli/publish_command.rs, replacing a detached blocking stdin reader thread and a global with a poll-based wait on the main thread, and adds PTY-based tests in test/cli/install/bun-publish.test.ts. It touches auth-flow UX (web login token retrieval) but no credential handling itself. The second commit resolves four of the earlier inline threads as verified from the code, while the poll gate and the blocking fd 0 read are unchanged; the remaining complexity is platform-specific tty/poll behavior that did not run on macOS or Windows, so a human review is still warranted.
| while taken < line.len() { | ||
| let left = deadline.saturating_duration_since(Instant::now()); | ||
| match self.readable_within(left) { | ||
| Some(true) => {} | ||
| Some(false) => return Waited::Nothing, | ||
| None => return Waited::EndOfInput, | ||
| } | ||
| match bun_sys::read(self.fd(), &mut line[..block]) { | ||
| Ok(0) => return Waited::EndOfInput, | ||
| Ok(len) if strings::contains_char(&line[..len], b'\n') => return Waited::Enter, | ||
| Ok(len) => taken += len, | ||
| // Another reader of the terminal took the line first. | ||
| Err(err) if err.is_retry() => return Waited::Nothing, | ||
| Err(_) => return Waited::EndOfInput, | ||
| } | ||
| } | ||
| // This many bytes without a line end are not ENTER, and /dev/zero never has one. | ||
| Waited::EndOfInput |
There was a problem hiding this comment.
🟡 nit (optional): a user who pastes more than 1024 characters before pressing ENTER on a terminal gets no page, and the tail of that line reaches the next prompt. In wait the Terminal arm reads 1024-byte blocks and the loop at src/runtime/cli/publish_command.rs:187 stops once taken reaches 1024 and returns Waited::EndOfInput, leaving the rest of the line and its \n in the tty queue. Fix: for a terminal keep reading until the line end or EAGAIN (the line discipline already caps a line at 4095 bytes), or size the buffer to that cap, so the whole line is consumed before the offer ends; the 1024 cap can stay for the pipe arm.
Why this was flagged
On a Linux or macOS terminal in canonical mode, the user pastes a long string (over 1024 bytes, e.g. a copied URL with query text) and presses ENTER during a 202 retry wait, so get_otp calls EnterOffer::pressed_before at src/runtime/cli/publish_command.rs:1115. wait at :178 uses block = 1024 for Self::Terminal (:182); the first read at :194 returns 1024 bytes with no newline, taken becomes 1024 (:197), the while condition at :187 fails and :204 returns Waited::EndOfInput. pressed_before then sets the offer to None (:214) and returns false, so open_in_browser is never called and the remaining bytes plus the \n stay in the tty input queue. The next reader of fd 0 (a postpublish prompt, or the Enter OTP: prompt under --recursive at :1163) receives that tail as its answer. On the base branch press_enter_to_open_in_browser looped on read(0) with a 1024-byte buffer until a chunk contained \n, so the whole line was consumed and the page opened. The comment at :203 says this bound is for /dev/zero, which only applies to the Stdin arm.
Verification: nit — triggered when stdin is a canonical-mode terminal and the user pastes a line longer than 1024 bytes before pressing ENTER during a 202 retry wait. In src/runtime/cli/publish_command.rs the read at :194 returns 1024 bytes with no \n, the while taken < line.len() loop (:187) exits and :204 returns Waited::EndOfInput. On the base branch the thread kept reading until it saw \n, then opened the page.
| function untilWaitingToRetry(pid: number) { | ||
| const waits = process.arch === "arm64" ? /^(73|101|115) / : /^(7|35|230|271) /; | ||
| return until(() => waits.test(readFileSync(`/proc/${pid}/task/${pid}/syscall`, "utf8"))); |
There was a problem hiding this comment.
🟡 nit (optional): on a Linux runner where the kernel refuses ptrace-style access to a child (kernel.yama.ptrace_scope=2, a hardened seccomp or LSM profile), the six untilWaitingToRetry tests fail with EPERM instead of testing anything. readFileSync of /proc/<pid>/task/<pid>/syscall at test/cli/install/bun-publish.test.ts:65 needs PTRACE_MODE_ATTACH on bun, and the throw escapes until straight into the test. Fix: catch the read error and fall back to a signal that needs no ptrace access (for example poll the 202 request count on the mock, or skipIf the first read throws with a stated reason) so the login tests still assert the ENTER wait on such runners.
Why this was flagged
Trigger: the Linux CI or developer machine sets kernel.yama.ptrace_scope=2 or runs the suite under a container profile that denies PTRACE_MODE_ATTACH to child processes. untilWaitingToRetry at test/cli/install/bun-publish.test.ts:63-66 calls readFileSync(/proc/${pid}/task/${pid}/syscall) on every 1 ms tick; the kernel's lock_trace check returns EPERM, readFileSync throws inside the until callback at :54-56, and the promise rejects. The six tests that await it ("ENTER pressed while bun waits ..." at :676 and the five "another reader of the terminal" runs at :769) fail with an EPERM error rather than a statement about bun publish. On the base branch these tests do not exist, so nothing in the suite depends on /proc ptrace access; after the merge the Linux lane of bun-publish.test.ts is red on such a runner regardless of whether the ENTER wait works. No skipIf or try/catch guards the read.
Verification: nit. Trigger: a Linux host where the kernel denies PTRACE_MODE_ATTACH to the test process for its child. test/cli/install/bun-publish.test.ts:65 readFileSync of /proc/${pid}/task/${pid}/syscall throws EPERM. until at :55-57 has no try/catch, so the rejection propagates to the awaiting test. Callers :690 and :790 are gated only by skipIf(!isLinux). Base branch has no such dependency.
Problem
postpublishorEnter OTP:question. From a pipe it takes 1024-byte blocks.press_enter_to_open_in_browser(src/runtime/cli/publish_command.rs:883) loops onread(0)and nothing stops it.Fix
EnterOffer). No reader of stdin outlives the login. The thread andWEB_LOGIN(install: fix regressions and older bugs found after the package manager consolidation #45307) are deleted.test/cli/install/bun-publish.test.ts, 17 new cases, main fails 5. Self-reviewed: 10 concerns raised, 9 addressed (not:/dev/nullfor the opener's output).Background
(press ENTER to open in browser). It asks the done url for the token. 202 means: ask again afterretry-after.echo "" | bun publish, so a maintainer must decide. A pipe's first line is still ENTER.Downsides
readfor the next one. On main it went on.start#26231).Notes
How this was found. By reading the code during the review of #36369, which lists it under "Not changed here". #46192 names it too. No user reported lost input after a web login. This change lands before both.
publish_command.rs:895.:892.bun login, for Nativebun login/bun logout#41289) moves this code intoweb_login.rsand can take the wait with it.What #45307 fixed is kept. Under
-r, ENTER opens the page of the package that is being published. Each login now has its own offer, so there is no state between two logins. Both test rows of #45307 are unchanged and pass.Rules of the offer
start, its opener, is not a program thatCreateProcessWcan run, so ENTER never opened a page there./dev/zerocannot hold up the login./dev/null).pollcannot watch the terminal (macOS, stdin is the/dev/ttyalias), there is no offer. This is by source and did not run.ThreadSpawn: failed to spawn thread for opening auth urland the login goes on. main stopped the publish there.Not changed here
printf 'for the script\n' | bun publish, and the login has to wait.postpublishstill reads 0 of 15 bytes, and the opener starts.OUTPUT=$(echo "" | bun publish ... 2>&1)to open the page. "No ENTER without a terminal" breaks them, so it is a decision for a maintainer. npm has that rule (lib/utils/open-url.js:62of npm 11.16.0), and the first comment of Nativebun login/bun logout#41289 asks for it. It is about 15 lines on top of this change.-r, an earlier web login that has to wait takes a piped one-time password as ENTER. It is the same decision.Enter OTP:question takes up to 4096 bytes of a piped stdin (BufferedStdin). Delete BufferedStdin: prompt(), bun init and the publish OTP question take one line of stdin #36369 owns that.xdg-openstarts on a machine without a display takes the terminal.AsyncHTTP::send_synchas no timed wait, so bun does not look for ENTER during a request. registry.npmjs.org answers in about 0.2 s with 202 andretry-after: 3.The read of fd 0. A pipe has one open file description, which every process that inherited it shares.
O_NONBLOCKon it would change the reads of those processes too, so bun reads it in blocking mode afterpollsays that a byte is there. The read waits only when another process takes that byte between the two calls. The same holds for a terminal that bun cannot open a second time.Measurements (release builds, linux x64, merge base 179664e against this branch)
postpublishquestion after a login without ENTER,firstandsecondtypedsecond(20 of 20)first(20 of 20)-r,Enter OTP:after a web login, two lines typed\nfor the script\n, login waitspostpublishreads 0 of 16 bytes (20 of 20)open the page\nfor the script\n, one wait of 0 msfor the script\n(20 of 20)for the script\n, done url answers at onceclone3,mmap,mprotect, 2rt_sigprocmask, 3clock_nanosleep, and 8 in the thread, which stays inread(0).ioctl,fstat,readlink,newfstatat,openat, 5poll,close. Threads started: 1 to 0.clock_nanosleep, 1poll, 1readthat returns 0).read(0)0 to 0,poll0 to 0./proc/<pid>/task/*/syscall, 3 runs each): tasks inread(0)1 to 0. Open fds: 9 to 10 during the login, 9 to 9 at the PUT with the OTP.get_otp(gdb hit counts on themi_*entry points, 3 runs each): 56 to 51.size -A:.text82,164,617 to 82,165,641 (+1,024),.rodataequal,.bss1,806,328 to 1,806,264 (-64). Symbols ofpublish_command(nm -S): 41,838 to 42,344 bytes.Tests
New cases:
retry-after: 30, the page opens, exactly one done request.postpublishquestion after the login./dev/zero).-r, typed and piped.A release build of main fails 5 of the 17 in every run. It fails a sixth (ENTER during the request that returns the token) in about half of the runs.
Each clause of the change has a case that fails without it. Checked with 10 modified builds:
pollwith timeout 0retry-afterignoredThe two cases for a blocking read of the terminal are a race. A single run fails in 12 of 20, so the case runs 5 times.
Unchanged and passing:
stdin: "\n"-rrows of install: fix regressions and older bugs found after the package manager consolidation #45307The 8 snapshot rows that print the offer check the heading of Windows and then compare with the snapshot of the other platforms.
Results:
bun-publish.test.ts: 296 pass on the debug build, and on the release build in 5 of 5 runs.cargo checkfor x86_64-pc-windows-msvc and aarch64-apple-darwin, clippy forbun_runtime, the source lints that read this file.-r, which runs through ConPTY.Probe on the debug build.
bun publish &in an interactive bash, keys typed at the prompt, thenfg: completes, 6 of 6. SIGTTIN stops the job in 3 of 6, and it resumes.no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/install/bun-publish.test.ts