Repository navigation
Conversation
The `bun update -i` picker hides the cursor and enables SGR mouse reporting, and the `bun init` template menu hides the cursor. Both undid that in a scope guard, which a SIGINT/SIGTERM exit never runs: the exit-signal handler wrote the startup termios back and re-raised, leaving the shell typing mouse escape sequences with no cursor. Track the active DEC private modes in one atomic (bun_core::tty) and have bun_restore_stdio(), the hook every exit path already goes through, write the resets with a plain write(2). The pickers set the modes through a DecModesGuard that registers them there. `bun --watch` installs its own SIGINT handler that _exit(0)s from signal context. It skipped the termios restore entirely, so a watched script in raw mode left the shell raw. Restore before the _exit.
|
Warning Review limit reached
On-demand reviews are free for the next 12 days. After that, they cost $0.25 per reviewed file. Or wait 1 minute for your next included review. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (9)
Comment |
|
Updated 7:02 AM PT - Sep 8th, 2026
❌ @robobun, your commit 3b805b2 has 1 failures in 🧪 To try this PR locally: bunx bun-pr 41977That installs a local version of the PR into your bun-41977 --bun |
|
Status: ready for review. CI pending. Reproduced on bun 1.4.3 (Linux x64) under a pty: Windows console Ctrl+C (#30890) checked by hand on Server 2019 with |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
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.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟣
src/runtime/cli/filter_run.rs— pre-existing:bun run --filterstill writes?2026h/?2026las raw bytes intodraw_bufviaOutput::SYNCHRONIZED_START/END, bypassingDecModesGuard/ACTIVE_DEC_MODES; a SIGINT/SIGTERM landing duringwrite_allof a large redraw frame leaves the terminal in synchronized-output mode, same as the class this PR fixes forOutput::synchronized(). Fix: recorddec::SYNCHRONIZED_OUTPUTinACTIVE_DEC_MODESaroundflush_draw_buf()(or route through aDecModesGuard) sobun_restore_stdio()writes?2026l; if intentionally excluded, say so in the PR per REVIEW.md.Extended reasoning...
redraw()(src/runtime/cli/filter_run.rs:508-648) buildsdraw_bufstarting with\x1b[?2026h(line 514) and ending with\x1b[?2026l(line 640), then callsflush_draw_buf()->write_allon stdout. With many filtered scripts producing output,draw_bufcan be tens of KB, requiring severalwrite(2)calls to the tty. A SIGINT delivered after the first write (which contains?2026h) but before the last (which contains?2026l) reachesonExitSignal->bun_restore_stdio()->Bun__ttyResetActiveDecModes(), butACTIVE_DEC_MODESis 0 because this path never touched it, so no?2026lis written and the emulator stays in deferred-render mode until its own timeout. On the base branch this behaves identically (pre-existing), but the PR's stated purpose is to route every DEC-mode flip through the atomic so the exit hook resets it, and its Notes section explicitly claims?2026coverage — this sibling writer of the same escape is not covered and not called out as excluded.Verification: pre-existing.
src/runtime/cli/filter_run.rsis not in the diff (verified:git diff --name-only a3e0ab60..HEADdoes not list it), and on the base branch nothing resets?2026on signal either — merging makes this path no worse. The finding is nonetheless the same bug class this PR fixes, and REVIEW.md requires "Fix the whole class in the same PR … every sibling site sharing the pattern".…
|
@robobun please resolve the merge conflicts |
|
Overlap note: #46248 adds a one-row version of this table for the bracketed paste mode of |
Problem
bun update -ihides the cursor and enables SGR mouse reporting (ESC[?25l ESC[?1000h ESC[?1006h). A SIGINT or SIGTERM sent to the pid exits throughonExitSignal(src/jsc/bindings/c-bindings.cpp), which writes the startup termios back but neverESC[?25h ESC[?1000l ESC[?1006l. The shell then types^[[<0;41;12Mon every click, with no cursor. Same for thebun initmenu's?25h, and for a console Ctrl+C on Windows (Cursor does not reset when pressingCtrl+Cduring interactive update #30890).bun --watch <script>+ SIGINT calls_exit(0)fromBun__onPosixSignal(src/jsc/PosixSignalHandle.rs:57, since node compat batch: callback-throw dispatch, Assert class + native deep-equality parity, Intl gate + URL/buffer fallout, compile cache, watch kill-signal, profilers (+98 tests) #34660). That handler replacedonExitSignal, so nothing wrote termios back: a watched script in raw mode left the shell-echo -icanon -isig.Fix
bun_core::tty::DecModesGuardsets the modes, records them in oneAtomicU8, resets them on drop.reset_active_dec_modes()writes the resets for what is still recorded with a plainwrite(2), from insidebun_restore_stdio()andwindows_stdio::restore(), the hooks every exit path already calls (the Windows console Ctrl+C handler too).Output::synchronized()use the guard. The--watchSIGINT exit callsstdio::restore()before_exit(0).update_interactive_formatting.test.ts,init/init.test.tsandwatch/watch.test.tsfail on 1.4.3 and pass here. The three files pass in full. Windows Ctrl+C checked by hand (Notes). Self-reviewed: 4 concerns raised, 4 addressed.Background
tcsetattrrestores. Cursor visibility and mouse reporting are DEC private modes: terminal emulator state that only a writtenCSI ? n h/lchanges.onExitSignalis Bun's SIGINT/SIGTERM handler: restore termios, re-raise. In raw mode Ctrl-C is a byte the picker reads, so only outside signals take this path.bun --watchowns SIGINT for node parity: the watcher exits 0.Fixes #30890. Supersedes #30891.
Notes
reset_active_dec_modes()is async-signal-safe: an atomic swap, a 64-byte stack buffer,write(2)with SIGTTOU blocked (a background job on a TOSTOP terminal gets EIO instead of a stop). It only writes when stdout is a terminal, so aFORCE_COLORpipe consumer never gets a blocking write from a signal handler.DecModesGuard::dropflushes before it clears the bits. Until the reset bytes reach the terminal a signal still finds the modes recorded. A reset written twice is harmless.ENABLE_PROCESSED_INPUT, so a console Ctrl+C raisesCTRL_C_EVENT.CtrlhandlercallsBun__restoreWindowsStdio, which now writes the resets withWriteFilebefore the default handler ends the process. Checked by hand on Windows Server 2019: a sibling process in the same console readsGetConsoleCursorInfoafterGenerateConsoleCtrlEventendsbun initat the menu.bVisibleis true with this change and false on 1.4.3. ConPTY output cannot show this (conhost re-renders and emits its own?25l/?25h, and it does not turn a written\x03intoCTRL_C_EVENT), so there is no automated Windows test.sigactionhandler (POSIX) andSetConsoleCtrlHandler(Windows), each with a hardcoded restore string and its own_exit. This change keeps one handler. The modes are data that the existingonExitSignal->bun_restore_stdio()funnel reads, so the crash handler,Global::exit,reload_processand the Windows Ctrl handler are covered by the same line, and a new widget only has to hold a guard.--watchhunk is an independent regression from node compat batch: callback-throw dispatch, Assert class + native deep-equality parity, Intl gate + URL/buffer fallout, compile cache, watch kill-signal, profilers (+98 tests) #34660 (the watch-mode SIGINT fast exit). It rides along because it is the same class (an exit path that skips the termios restore) and uses the same callonExitSignalalready makes from signal context.bun_restore_stdio()/onExitSignalregion. The overlap is textual only (adjacent lines); whichever lands second needs a one-line rebase.?2026(synchronized output):Output::synchronized()now records the mode per frame, so a kill in the middle of a frame also writesESC[?2026l. The render loop already flushed after every frame, so the flush in the guard's drop adds no write.test/js/bun/terminal/terminal-spawn.test.ts,update_interactive_snapshots.test.tsandupdate_interactive_install.test.tslocally (Linux x64 debug+ASAN).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/watch/watch.test.ts, test/cli/update_interactive_formatting.test.ts, test/cli/init/init.test.ts