Skip to content

Embed shared terminal interaction in the TUI - #73

Merged
nicodes merged 2 commits into
mainfrom
feature/507-tui-terminal-screen
Aug 25, 2026
Merged

Embed shared terminal interaction in the TUI#73
nicodes merged 2 commits into
mainfrom
feature/507-tui-terminal-screen

Conversation

@nicodes

@nicodes nicodes commented Aug 25, 2026

Copy link
Copy Markdown
Owner

Closes nicodes/ormos-be#507

Summary

  • replace the blocking tea.Exec handoff with a terminal screen owned by the existing Bubble Tea program
  • emulate replay and live PTY output with tagged MIT dependency github.com/vito/midterm@v0.2.5, so PTY cursor, erase, SGR, and alternate-screen sequences cannot escape into Bubble Tea
  • reuse the existing TerminalClient and exact persisted projectID:sessionID key; shared PTY registry, browser transport, replay, scheduler, and expiry code are unchanged
  • serialize keyboard input, retry bounded backpressure for at most 2 seconds at 10 ms intervals, forward emulator device responses, resize from WindowSizeMsg, and intercept Ctrl-G before PTY input
  • retain TUI terminal header, connection status, and Ctrl-G: dashboard footer
  • remove the now-dead external raw-mode/stdin/SIGWINCH Exec path

Risk

Risk: high. This changes terminal input/output ownership, VT rendering, resize, and multi-client interaction. It does not change authentication, authorization, shared-session identity, browser wire behavior, or PTY registry ownership. Two independent reviewers are required on one unchanged exact green head.

Versioned references

Bubble Tea documents tea.Exec as pausing the Program. Its renderer rewrites a line-oriented View, so raw PTY control bytes cannot safely be painted directly. Midterm is an in-memory terminal emulator intended for terminals embedded in TUIs; Render emits screen cells and SGR rather than PTY cursor/OSC/erase commands.

Verification

  • focused terminal tests: 20 consecutive runs
  • focused race-enabled terminal tests: 5 consecutive runs
  • full go test ./... -count=1
  • full go test -race -count=1 ./...
  • go vet ./...
  • govulncheck ./...: zero reachable vulnerabilities; one required-module vulnerability is not on a called path
  • go mod verify: all modules verified
  • gofmt -l .: no output
  • git diff --check: no output
  • static builds: Linux and Darwin, amd64 and arm64, with CGO_ENABLED=0
  • Darwin internal/system test binaries: amd64 and arm64
  • unsupported controls: GOOS=windows GOARCH=amd64 and GOOS=js GOARCH=wasm select only github.com/nicodes/ormos/relay
  • control comparison: existing TestLocalTerminalClientSharesBrowserSession remains unchanged and passes in the full/race suites

Mutation evidence

Each mutation was applied alone, its focused test was run with -count=1, and the production implementation was restored before final verification:

  • removed Ctrl-G interception: TestTerminalScreenRendersOutputAndSerializesInput failed with Ctrl-G left mode 3
  • skipped emulator output ingestion: the same test failed with terminal view missing chrome or output and showed an empty 27-row screen
  • allowed concurrent queued writes: the same test failed with second input started before the first completed
  • sent rows+1 to the shared PTY on resize: TestTerminalScreenResizesSharedPTY failed with resizes = [][2]int{[2]int{100, 38}}
  • omitted stale-client detach: TestTerminalScreenIgnoresStaleAttachAfterDetach failed with stale attach mode=0 terminal=false detach=0
  • assigned unsanitized attach errors: TestTerminalScreenAttachFailureReturnsSafely failed while exposing the injected OSC sequence in m.err
  • removed backpressure retry: TestTerminalScreenRetriesBackpressureAndReturnsErrorsSafely failed with write attempts = 1

Known limitations

  • Bubble Tea v1 exposes parsed KeyMsg values rather than arbitrary stdin bytes. Common xterm keys, modifiers, function keys, controls, Unicode, and paste are encoded explicitly; unknown CSI and Kitty keyboard sequences cannot be recovered from the public API.
  • Inner-terminal mouse protocols and host clipboard/title side effects are intentionally not forwarded. The dashboard disables host mouse reporting while attached.
  • Sixel and Kitty graphics are not rendered. Text shells and VT full-screen applications are the supported surface.

@nicodes

nicodes commented Aug 25, 2026

Copy link
Copy Markdown
Owner Author

Initial high-risk review: GPT-5.6 Luna

Pinned exact head 022c3a2d190039ee34ab70c6127a013e52ca077a; inspected the complete origin/main...HEAD diff and versioned dependency behavior.

BLOCKING: bracketed paste delimiters are dropped

terminalKeyBytes ignores KeyMsg.Paste. Bubble Tea removes ESC[200~ / ESC[201~, so an inner shell/editor that enabled bracketed paste receives bare multiline input and may execute it immediately.

BLOCKING: application cursor mode is not honored

The encoder always emits normal cursor sequences and does not track DECCKM. An app requesting CSI ? 1 h receives ESC[A rather than ESC OA.

Residual gap: no real Bubble Tea TTY/full-screen app walkthrough yet. Mouse, Kitty keyboard, and unknown CSI remain stated limitations.

NEEDS ATTENTION

@nicodes

nicodes commented Aug 25, 2026

Copy link
Copy Markdown
Owner Author

Initial high-risk review: Grok 4.6

Pinned exact head 022c3a2d190039ee34ab70c6127a013e52ca077a; inspected the complete eight-file diff and confirmed shared-session/browser control files are byte-identical to origin/main.

BLOCKING: superseded live attachment leaks

A later terminalCreatedMsg can call startTerminal while another attachment is live. Replacing m.term without Detach() leaves an unreachable local client, prevents detached-session expiry, and can consume the four-client limit.

FOLLOW-UP: clamp emulator height

Midterm VPA/IL handling can grow Height beyond the pane, causing Bubble Tea to retain bottom rows and hide chrome. Assert fixed view height after hostile VPA/IL output.

FOLLOW-UP: test device responses

ForwardResponses is wired but DA/DSR bytes are not asserted through the serialized PTY write path.

No other blockers. PTY resize argument order, Ctrl-G interception, stale generation handling, backpressure, and unchanged browser controls were verified.

NEEDS ATTENTION

@nicodes

nicodes commented Aug 25, 2026

Copy link
Copy Markdown
Owner Author

Review-wave disposition

All findings are accepted and will be remediated together before a new review head:

  • BLOCKING dropped bracketed-paste delimiters: track the inner mode and wrap only KeyMsg.Paste while enabled.
  • BLOCKING unhonored DECCKM: track the inner mode and emit SS3 arrows for unmodified application-cursor keys.
  • BLOCKING superseded attachment leak: detach any current client before replacing terminal state; add a completed-attach/later-create regression.
  • FOLLOW-UP emulator height growth: fix now by restoring the emulator to the pane grid after output and assert chrome survives VPA/IL growth.
  • FOLLOW-UP device responses: fix now with exact DSR/DA response ordering coverage.

The blockers and follow-ups will be batched into one remediation commit. Both original reviewer contexts will verify their findings and the complete remediation delta after the new exact head is green.

@nicodes

nicodes commented Aug 25, 2026

Copy link
Copy Markdown
Owner Author

Remediation pushed

New head: bed53b0033f8ad2a613e56afbcad3f0fb4981694. The complete 022c3a2..bed53b0 remediation batch:

  • detaches any live client before a later terminal start replaces model state
  • consumes midterm-forwarded DECCKM and bracketed-paste mode changes locally, without forwarding terminal control side effects to the host
  • emits SS3 arrows for unmodified application-cursor keys and wraps KeyMsg.Paste only while bracketed paste is enabled
  • clamps active/alternate screen dimensions plus active/saved cursors after hostile VT growth and normal resize
  • proves DSR/DA responses are serialized behind an in-flight user key

Remediation verification:

  • focused tests: -count=20
  • focused race tests: -count=5
  • full tests and full race tests: -count=1
  • vet, formatting, diff check, and module verification passed
  • govulncheck ./...: zero reachable vulnerabilities
  • Linux/Darwin amd64+arm64 static builds and Darwin test-binary builds passed

Each remediation gate was deliberately broken alone and restored:

  • omitted superseded detach: superseded terminal detach=0 ...
  • omitted inner mode forwarding: enabled modes cursor=false paste=false
  • omitted paste wrappers: bracketed paste = "echo one\necho two"
  • omitted DECCKM encoding: application cursor = "\x1b[A"
  • omitted emulator clamp: emulator grew to 80x1998
  • omitted device responses: serialized input = ["x"]

Both original reviewer contexts will verify this exact head only after CI is green.

@nicodes

nicodes commented Aug 25, 2026

Copy link
Copy Markdown
Owner Author

Exact-head remediation approval: GPT-5.6 Luna

Verified exact target bed53b0033f8ad2a613e56afbcad3f0fb4981694, initial head, complete current diff, and remediation delta.

Bracketed-paste and DECCKM tracking, superseded-client detachment/generation filtering, active/alternate emulator clamping, and serialized device responses are fixed and covered by reachable mutation tests. No remediation-caused or newly exposed material defects were found.

Residual risk is limited to the documented absence of a real-TTY/full-screen integration walkthrough; fake-attachment and mutation coverage directly exercise the remediated paths.

APPROVE

@nicodes

nicodes commented Aug 25, 2026

Copy link
Copy Markdown
Owner Author

Exact-head remediation approval: Grok 4.6

Pinned and verified bed53b0033f8ad2a613e56afbcad3f0fb4981694. The remediation is one commit touching only tui_terminal.go and tui_terminal_test.go.

All prior findings are fixed: superseded live clients detach; hostile VPA/IL growth clamps to 80x27 while preserving 30-line chrome; DSR/DA writes remain behind in-flight user input; DECCKM emits SS3 arrows; bracketed paste wraps only while enabled. Each assertion was proven reachable through a deliberate failing mutation.

No findings and no remediation-caused or newly exposed material defects. Residual limitations remain the documented fake-attachment test surface, unrecoverable Kitty/unknown CSI, and wide-cell rendering.

APPROVE

@nicodes
nicodes merged commit 8270c5b into main Aug 25, 2026
4 checks passed
@nicodes
nicodes deleted the feature/507-tui-terminal-screen branch August 25, 2026 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant