Repository navigation
feat(terminal): interactive PTY shell in the embedded terminal - #406
Merged
Merged
Conversation
The terminal ran each line as its own `sh -lc`, killed it after 30 s and kept only the last 4,000 characters of output, so a dev server, a log follower or an interactive prompt could not be used. It also always started in /root, whatever workspace it was opened from (#329). It now keeps one shell alive for as long as the screen is open: - PTY: the sandbox's `script -qefc` allocates the pty (the same trick the Claude Code and Antigravity sign-in flows use), so the shell has a real controlling terminal - line editing, password prompts and Ctrl+C work. `stty` sets the size the screen measured, since `script` otherwise reports 0x0 over pipes. - Streaming: output is read as it is produced and interpreted by AnsiTerminalEmulator, a line-oriented VT100/xterm subset (SGR colours, \r rewrites, line editing, clear, wrapping, 10,000 lines of scrollback). Unknown sequences are dropped instead of printed. - Input: the typed line is sent with Enter; Esc, Tab, Ctrl+C, Ctrl+D and the arrow keys have their own key row. - Starts in the workspace it was opened from, falling back to /root; the workspace folder is bound at /workspace. - A runtime work lease is held while the shell lives so the device does not suspend under a build. The runtime access lock is held only while the process is spawned, so a long-lived shell cannot stall an update. - The shell and anything it started end when the screen is left. Full-screen programs (vim, htop), resizing an open shell and the multi-terminal tab placeholder are not part of this change. Closes #329 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ut parser Pre-PR review of the interactive terminal found two blockers and several smaller problems. Blockers: - Closing the screen only called destroyForcibly() on proot. A proot guest outlives that (the repo's own on-device finding, ProcessTree.kt): the `script`/`bash` and any dev server stayed running while the runtime lease was released. close() now walks /proc and kills the tree end first (killProcessTree, renamed from killAntigravityProcessTree since it is not Antigravity specific) on a background thread, then destroys the root even if the walk failed. - A CSI parameter such as ESC[2147483647@ overflowed Int arithmetic in the emulator and threw out of the collector, crashing the app from any `cat` of a hostile file. Numeric parameters are clamped to 9,999, a feed() failure is logged instead of propagated, CAN/SUB abort an escape sequence in progress, and the interned style table is bounded. Also fixed: - A shell that finished opening after the screen was closed was never owned by anyone and kept running; it is now ended on arrival. - The emulator was sized from the raw measured width while the pty was clamped to 20..240 columns; both use the same bounds now. - Tab and the arrow keys act on the shell's line editor, so text still in the input field is sent first. - Output following stopped when the soft keyboard shrank the viewport (it judged "at the end" by canScrollForward at that moment). It now follows until the reader drags away and resumes when they drag back to the end. - State publishes happen under the emulator lock so a late snapshot cannot bring back text that `clear` removed. - Removed the now unused terminal_banner / terminal_exit_code strings from all locales; documented the remaining limits (CJK width, OpenCode restarts ending the shell, the lease being held while a shell is open). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
- close() no longer walks the process tree of a shell that already ended: its pid has been reaped and may belong to something else. - The output safety net now also covers snapshot(), and a CSI sequence that carries a negative parameter is ignored whole (ESC[-1m used to be read as ESC[0m and reset the colours). - Typing, pressing a key and clearing bring the view back to the end of the output after the reader scrolled away. Scrolling away by hand still stops the following; mouse-wheel and TalkBack scrolling do not, which is written down in the docs rather than guessed at. - Documented that Tab and the arrow keys send the pending input first. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
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.
Summary
Closes #329. The embedded terminal is now an interactive shell on a real PTY instead of one bounded
sh -lcper line.Before: every submitted line ran as its own
sh -lcthat was force-killed after 30 s, only the last 4,000 characters of output were read back, there was no stdin / streaming / Ctrl+C, and it always started in/rootregardless of the workspace it was opened from.Now: one shell stays alive for as long as the terminal screen is open.
LocalTerminalLauncher): the sandbox's/usr/bin/script -qefcallocates the pty — the same mechanism the Claude Code and Antigravity sign-in flows already use. The shell gets a real controlling terminal, so line editing, password prompts (no echo) and Ctrl+C (the line discipline turns0x03into SIGINT for the foreground job) behave.scriptreports a 0x0 window when fed pipes, sostty rows/colssets the size the screen measured before the shell starts (clamped to the same bounds the emulator uses).ProcessTerminalSession): read as it is produced, batched per burst; no timeout and no 4 KB cap.AnsiTerminalEmulator, plain Kotlin): a line-oriented VT100/xterm subset — SGR colours (16 / 256 / truecolour, bold, italic, underline, inverse),\rrewrites (progress bars),\b, tabs, erase line/display, cursor movement, insert/delete characters,clear, auto-wrap, and 10,000 lines of scrollback. Sequences it does not know are consumed, never printed. The stream is treated as untrusted: numeric parameters are clamped, sequences with invalid parameters are ignored, CAN/SUB abort a sequence in progress, and the style table is bounded.TerminalScreen): the typed line is sent with Enter (an empty line too);Esc,Tab,^C,^Dand the arrow keys have their own key row that always fits the screen width. Tab and the arrows send the text still in the input field first, since they act on the shell's line editor. Output is followed until the reader scrolls away by hand, and again when they scroll back, type, press a key or clear.terminal?directory=…, falling back to/rootifcdfails); the workspace folder is bound at/workspace.killProcessTree(renamed fromkillAntigravityProcessTree, which is not Antigravity specific) walks/procand kills the guest tree, because killing the proot root alone leavesscript/bashand any dev server running (see the on-device finding inProcessTree.kt). ARuntimeWorkTrackerlease is held only while the shell is alive so the device does not suspend under a build or dev server. The runtime access lock is held only while the process is spawned — the lock is fair, so keeping it for a shell that may stay open for hours would make the next runtime update (and every command queued behind it) wait for the user to close the terminal.terminal_banner/terminal_exit_codestrings are removed from all of them); README anddocs/LOCAL_RUNTIME.mdupdated.Not in this PR
vim,htop): no alternate screen, scroll region or mouse. The cursor-position query (ESC[6n) is not answered.sttyagainst the pty slave from a second process.)canScrollForwardat the wrong moment is what broke following when the keyboard opened, so it is deliberately not generalised blindly.TerminalTabPlaceholder) — mentioned in the issue as context, not as an expectation.Test plan
./gradlew spotlessCheck :app:testGithubDebugUnitTest :app:lintGithubDebug detekt— green on the final commit: 1562 unit tests, 0 failures (8 skipped = pre-existing liveOpenCodeV2LiveTest), lint passes. Note the rootdetekttask reportsNO-SOURCEin this repo, so it checks nothing.AnsiTerminalEmulatorTest(36: wrap/deferred wrap,\r, erase/insert/delete, cursor clamping, colours incl. sequences split across chunks, scrollback cap, immutable snapshots, overflow/negative/CAN/style-bound hardening),LocalTerminalLauncherTest(8: command shape,sttysize and bounds,cdquoting/injection, relative path ignored),ProcessTerminalSessionTest(14, against a realshover pipes: streaming, input order, exit code, UTF-8 both ways, close ends the tree before the root, a failing tree walk still destroys the root, close does not block, an ended shell is not walked),TerminalViewModelTest(24: size handshake and clamping, streaming, submit/keys incl. flushing typed text, drain-before-exit, lease acquire/release, close, a shell opened after close is ended, failures, restart, clear),RoutesTest(+2: terminal route round trip).TerminalScreenInstrumentedTest(realTerminalScreen, 12 cases: reports a usable measured size, draws output, key sequences, typed input, disabled until running, ended / not-installed messaging, restart, clear, follows output, keeps following when the keyboard shrinks the viewport, stops when the reader scrolls up and resumes at the end, input returns to the end) — 12/12 on an API 36 emulator. Also looked at a screenshot of the screen driven by real emulator output (colours, bold, inverse, underline,\r\e[2Krewrite, Japanese text, block cursor, 54 columns measured on a 411 dp-wide display); that surfaced the key row overflowing the width, fixed here..github/workflows/i18n-check.yml(ported from GNUgrep -P), 1152 required keys, all 7 translations complete.script: input is echoed,^Cinterrupts a foregroundsleep 30and the shell survives, and the pty reports0 rows; 0 columnsunlesssttysets it — the reason for thesttyin the launcher.script -qefc … bash -linside proot,Ctrl+Cthrough it, and the whole-tree kill on leaving the screen are exercised only through the unit tests (a realshover pipes, the tree kill behind an injected function) and the macOS check above. Worth a quick manual pass: open a workspace terminal, runpwd,read -p x y,sleep 100+^C, start a dev server, then leave the screen and confirm withpsfrom another terminal that nothing is left.Pre-PR review
The named
repo-reviewersubagent (.opencode/agents/repo-reviewer.md) is not registered in Claude Code, so the review was run by a read-only general-purpose subagent given that definition. Two rounds:e4b236f8→ REQUEST_CHANGES with two blockers (the shell's process tree was not ended on close; an absurd CSI parameter overflowed and crashed the app). Both are fixed in106fc24d, together with the suggestions that mattered (a shell opened after the screen was closed leaked; emulator/pty size bounds; Tab/arrows vs typed text; output following broke when the keyboard opened; publish ordering; dead strings; docs).106fc24d→ APPROVE (report below, verbatim).Commit
4c917a06was added after the approval and applies only suggestions from that report (no walk of an already-ended shell's tree,snapshot()inside the safety net, negative CSI parameters ignored, input/keys/clear return to the end of the output, docs). It was not re-reviewed.🤖 Generated with Claude Code