Swap vt10x for charmbracelet/x/vt, add mouse support to the split-pane TUI by cameronsjo · Pull Request #3 · citizen-123/cli-capture

cameronsjo · 2026-07-27T22:29:24Z

Twelve commits: a terminal-emulator swap that fixes truecolor, full mouse support in the
split-pane TUI, and a handful of first-run and layout fixes. Each commit stands alone and
carries its reasoning in the message, so this reads best commit-by-commit.

What's here

Emulator swap — hinshun/vt10x → charmbracelet/x/vt

vt10x has been unmaintained since 2022 and packs 24-bit truecolor into the same numeric
space as 256-color palette indices, so truecolor targets silently lost color. x/vt carries
a real color.Color per cell, so Render now walks CellAt and uses Style.Diff to emit
only the SGR delta between adjacent cells instead of the hand-rolled sgr()/colorSGR().
x/vt's Read is pull-based, so NewVT starts a pump goroutine draining replies and
Close stops it cleanly rather than leaking it. The pump runs even when there is no reply
sink, since a query written into an undrained pipe blocks the emulator forever.

That also removed the last non-test caller of the old line-oriented Screen emulator, which
is deleted here along with its helpers and the package doc's warning that alt-buffer TUIs
would not render faithfully — VT has made that false since it became the only implementation.

Mouse support in the TUI

Clicks move pane focus and selection; clicking an already-selected flow row opens it (the
same path enter takes — bubbletea reports no click count, so click-again avoids hand-rolling
double-click timing). Mouse events reach the hosted child through a neutral
terminal.MouseEvent, so internal/tui never sees x/vt and internal/terminal never sees
bubbletea. x/vt's SendMouse already gates on the child having enabled a DECSET mouse
mode, so the TUI's own gating and hand-rolled SGR encoder went away.

A vertical wheel over the terminal pane sends PgUp/PgDn instead of forwarding notches. This
is not a translation bug being papered over: with mouse tracking on, the child receives real
SGR wheel reports and reasonably scrolls line-wise, which reads like tapping the arrow keys.
Clicks, drags, and the horizontal wheel still forward untouched, so a target's clickable UI
keeps working. There is also a leader command to toggle mouse capture at runtime, and
-leader now makes the leader key configurable.

Fixes

paneRects sized each pane as width/2-1 plus 2 border columns, but lipgloss borders
both sides of both panes — four columns. The rendered row came out width+2 at every
even width (80 → 82, 120 → 122), the terminal wrapped every pane line, and the layout slid
down the screen so mouse Y addressed the wrong row. The two boxes now sum to exactly
width, with the spare column of an odd width going to the right pane.
The log file was created before the config dir; capture file modes were wider than needed.
The detail view now expands embedded JSON and escaped newlines.
A Target constructed but never started no longer panics on a nil Pty — the shape several
existing tests build.

Test coverage comes along: the SGR mouse encoder is pinned against xterm's ctlseqs button-code
definition rather than by inverting bubbletea's decoder, so the two derivations have to agree
independently.

Disclosure: a data race in `charmbracelet/x/vt`

Worth knowing before you merge.

The dependency has no tagged release; go.mod pins the pseudo-version
v0.0.0-20260726004341-482a56510f1b.
go test -race ./... exits 1 on this branch. go build, go vet, and go test ./...
all exit 0.
The race is upstream, not in this code: Emulator.Close() (emulator.go:265) writes the
closed field while Emulator.Read() (emulator.go:252) reads it, unsynchronized.
It is reachable by design rather than by misuse — Close() is the only way to interrupt a
blocked Read(), so any correct shutdown of a reply pump triggers it. It surfaces here in
TestVTCloseStopsReplyPumpWithoutLeaking; no other package races.

I have not worked around it downstream, because any workaround would have to avoid calling
Close() on a blocked reader, which is the only shutdown the API offers. Fixing it means a
mutex or atomic on closed in x/vt. Happy to file it upstream against charmbracelet/x if
you would rather that go out under this project's name — or leave it to you, your call.

Notes

The branch is built directly on the current main (13d309c), so every commit references
the module path as it stands today and it applies with no history predating that commit.
Heads-up on overlap: resize-panes and worktree-repeater both touch internal/tui/model.go
and internal/tui/help.go, as does this branch. Whichever lands second will want a look —
happy to rebase this one if you would rather merge those first.

…e modes Two independent first-run/security defects found while smoke-testing on macOS. cmd/cli-capture/main.go opened <confDir>/cli-capture.log before ca.LoadOrCreate created confDir, and discarded the error. On a machine without ~/.cli-capture the os.Create failed silently, log.SetOutput never ran, and every log.Printf landed on the bubbletea alt-screen — defeating the intent stated in the comment directly above it. Move the open below ca.LoadOrCreate and fail loudly instead of discarding the error. The capture exports (capture.har, flagged.txt, flow-*.txt, flow-*.curl) and session.json hold Authorization headers and request bodies verbatim, but were written 0644. ca.key was already 0600, so this reads as an oversight. Narrow all five to 0600. os.MkdirAll does not chmod a directory that already exists, so a config dir the user created by hand stays 0755 and the mode on ca.key becomes the only thing guarding the captures. Chmod the dir to 0700 either way. Verified on darwin/arm64, Go 1.26.5: go build, go vet and go test ./... all exit 0. With ~/.cli-capture absent, a run now writes cli-capture.log and sprays nothing onto the TUI; with the dir pre-created 0755 it comes back 0700 and the exports land -rw-------.

vt10x packs a 24-bit color as r<<16|g<<8|b in the same uint32 space it uses for palette indices, so colorSGR's "16-255 is the xterm palette" branch swallowed every RGB color. An orange cell (255,100,0) came out as "38;5;16737280" — a palette index 16 million out of range. Terminals discard the parameter and fall back to the default foreground, which is why targets that use truecolor rendered as plain white text. Add the 256 <= c < 1<<24 branch. The existing colour test only asserted that some SGR escape survived, and a bogus index is still an escape, so it passed throughout; the new tests assert the actual parameters. One case stays wrong and is marked debt: an RGB triple that packs below 256 (near-black blues) is indistinguishable from a palette index in vt10x's encoding, and still renders as palette.

The tmux-style prefix was hardcoded to Ctrl+A, which collides with readline's beginning-of-line and with tmux's own default prefix. Since the leader is the one chord cli-capture takes away from the target app, it is exactly the key that should be movable. Add -leader, accepting ctrl+a … ctrl+z and ctrl+space. Across the ctrl+<letter> range a bubbletea KeyType is numerically the C0 byte that chord transmits, so one value drives both the match and the literal-passthrough write; a test pins that relationship in case bubbletea ever renumbers. Ctrl+Space and Ctrl+@ are the same NUL byte and resolve to one binding under the requested name. Three things had the old key baked in and would have made a rebind useless: the literal-passthrough write sent a hardcoded 0x01, the status line advertised Ctrl+A, and the help overlay's shortcut table did too. All three now follow the configured leader. A bad spec fails on stderr before the TUI takes the screen, naming the accepted forms. Verified end to end: with -leader ctrl+space, NUL+q quits at 7.1s, while Ctrl+A+q runs to the timeout untouched — the key is moved, not duplicated.

…erface The TUI is about to gain mouse forwarding into the left pane's child PTY, but blindly writing mouse escapes into a child that never asked for them would just inject garbage keystrokes. vt10x already tracks the DEC private mouse modes (1000/1002/1003) and the SGR (1006) coordinate encoding as part of its terminal state, so expose both through two new Emulator methods rather than reaching into vt10x from the tui package. Screen, the line-oriented fallback emulator, has no mode state and always answers false.

Tool-call arguments and MCP payloads carry JSON-in-a-JSON-string, and system prompts/model output carry text with escaped newlines — both are common in the LLM/agent traffic this tool targets, and json.Indent correctly leaves string contents untouched, so both render as one unreadable escaped line today. colorizeJSON now recognizes a string VALUE (never a key) that decodes to embedded JSON or to text containing "\n", and expands it inline, indented under its key, recursively (capped at maxEmbedDepth — captured traffic is adversarial by construction, so the cap bounds render time/stack depth against a malicious or malformed payload rather than following it arbitrarily deep). Every line of expanded content is prefixed with a distinctly-styled marker (jsonExpandStyle, not a reuse of the existing semantic JSON styles) so it can never be mistaken for what was literally on the wire. This is render-time only: prettyJSON/colorizeJSON take a new `expand bool` and build a new string from the already-captured bytes — the stored Flow/Message bytes are never touched, and internal/export/ (HAR, flow-*.txt, curl) is untouched and stays byte-exact. wrapDetail's signature is unchanged; it now calls renderFlowDetail(f, width, true), so a future UI toggle can call it with expand=false without any exported-path impact.

…lit panes Enable cell-motion mouse reporting and route tea.MouseMsg through a new paneRects()/contentGrid() pair that both View() and the mouse handlers call, so hit-testing can never drift from what's actually drawn. Clicking a pane focuses it; clicking a flow row selects it (mirroring renderTraffic's own reserved-row accounting via the shared trafficRowLayout helper, extracted so the render path and the click path can't disagree); the wheel scrolls the open detail viewport or walks the flow-list selection with the same clamping j/k already use. Left-pane events only forward into the child PTY when the child emulator reports both mouse tracking and SGR (mode 1006) encoding are on — writing mouse escapes into an app that never asked for them would just inject garbage keystrokes, and legacy X10 encoding is out of scope.

Capturing the mouse takes over the terminal, which kills the operator's native text selection and copy-paste — there was no way back short of restarting cli-capture. Wire a leader "m" command that flips a mouseCapture flag and returns tea.EnableMouseCellMotion/tea.DisableMouse to re-issue the DECSET/DECRST sequences live, with a status line naming whatever leader is configured (never a hardcoded chord) so the operator can find their way back. Quitting was already safe: bubbletea's own shutdown path disables mouse reporting unconditionally before it restores the terminal.

… decoder encodeSGRMouse was derived by inverting bubbletea's own parse path, which can faithfully reproduce that decoder's quirks instead of the wire format. These expectations come from xterm's ctlseqs button-code definition instead, so the two derivations have to agree independently: button bits, the 4/8/16 modifier bits, the 32 motion bit, the 64 wheel range, the 128 extended-button range, M-versus-m finals, and 1-based coordinates. Includes the case that would be silently wrong rather than obviously broken: a wheel event must never pick up the motion bit, since 96 decodes as a drag and the child would act on the wrong event entirely.

…into the emulator hinshun/vt10x has been unmaintained since 2022 and packed 24-bit truecolor into the same numeric space as 256-color palette indices, so truecolor targets silently lost color. x/vt's Style carries a real color.Color per cell (color.RGBA vs ansi.IndexedColor), so Render now walks CellAt and uses Style.Diff to emit only the SGR delta between adjacent cells instead of the old hand-rolled sgr()/colorSGR(). x/vt's Read is pull-based (an io.Pipe) rather than vt10x's push-based WithWriter, so NewVT now starts a pump goroutine draining replies (and forwarded mouse bytes) into resp; Close stops it cleanly instead of leaking it. The pump always runs, even with resp==nil, since a query/mouse write into an undrained pipe blocks the emulator forever. x/vt's SendMouse already gates itself on the child having enabled a DECSET mouse mode, so the TUI's own MouseEnabled/MouseSGR checks and hand-rolled encodeSGRMouse are no longer needed: internal/tui now translates tea.MouseMsg into a neutral terminal.MouseEvent and lets Emulator.ForwardMouse do the rest, keeping bubbletea out of the terminal package and x/vt out of internal/tui.

Screen predates VT and had zero non-test callers — main.go has wired NewVT since full-screen TUI support landed. It only still existed to satisfy the old Emulator interface. Drop it, its helpers (escape, isFinalCSI, clip, newline, backspace), and its tests, and rewrite the package doc: it described Screen as the design and warned that full-screen alt-buffer TUIs wouldn't render faithfully, which VT has made false since it became the only implementation.

…on click Two reports from using the merged build: clicking a pane didn't seem to take keyboard focus, and traffic rows couldn't be opened by clicking. The second was a plain gap; the first turned out to be a symptom of a pre-existing layout bug that adding mouse support made visible. paneRects sized each pane as m.width/2-1 of content and then added 2 border columns, but lipgloss puts a border on BOTH sides of BOTH panes — four columns, not two. Measured, the rendered row came out m.width+2 at every even width (80 -> 82, 120 -> 122, 200 -> 202). The terminal wrapped every pane line, which slid the layout down the screen, so mouse Y no longer addressed the row the user could see and clicks landed on the wrong row or on nothing at all. Size the two boxes to sum to exactly m.width instead, giving the spare column of an odd width to the right pane, and let View size each pane from its own box. The old geometry test asserted the panes must be exactly equal, which is the assumption that made the overflow unavoidable — an odd width cannot be split evenly without rendering a column too many. It now allows a one-column difference and asserts the sum instead. Clicking a flow row only ever set m.selected. Clicking the row that is already selected now opens it, the same path enter takes. bubbletea reports no click count, so click-again avoids hand-rolling double-click timing while keeping "move the selection without opening" available.

…otches Scrolling the terminal pane crawled a line or so per notch, which reads like tapping the arrow keys and takes forever to get anywhere in a long transcript. The cause is not a translation bug: x/vt does no alternate-scroll rewriting at all — with mouse tracking off it emits nothing for a wheel event, even with mode 1007 set — so the notch was reaching the child verbatim. Claude Code turns on ?1000h ?1002h ?1003h ?1006h, so it was receiving real SGR wheel reports and choosing to scroll line-wise. Faithful, and not what you want from a host. Send PgUp/PgDn for a vertical wheel over the terminal pane instead, so one notch moves a screenful. Only the vertical wheel is remapped: clicks, drags and horizontal wheel still forward untouched, so a target's clickable UI keeps working. Acting only on the press avoids paging twice when bubbletea reports a companion release or motion event for one physical notch. Also guards a nil Pty on a Target that was constructed but never started — the shape several existing tests build, and previously a panic on this path.

citizen-123 · 2026-07-27T23:21:26Z

Code review

Nice change — the emulator swap and mouse support are well done. Found 3 issues, all in the same "modal state gates keyboard but not mouse / a reserved key" family:

Mouse input isn't gated by the help overlay the way keyboard input is. Update dispatches tea.MouseMsg to onMouse before the tea.KeyMsg branch that checks m.showHelp, so while help is open (and View renders only the help box) a click still hit-tests the underlying split-pane geometry: a left-pane click forwards a synthetic mouse event into the hidden child process and sets focus, and a right-pane click changes the traffic selection or opens the detail view — all invisible behind the overlay.

cli-capture/internal/tui/model.go

Lines 278 to 284 in 2941f9d

    
           case tea.MouseMsg: 
        
           	return m.onMouse(msg) 
        
           case tea.KeyMsg: 
        
           	if m.showHelp { 
        
           		return m.onHelpKey(msg)

Wheel-scroll over the right pane ignores m.editing. The click path guards with if m.editing || m.viewing, but the wheel is dispatched earlier and onRightPaneWheel only checks m.viewing. Scrolling while the raw-request editor is open therefore moves the background traffic-list selection instead of scrolling the editor.

cli-capture/internal/tui/mouse.go

Lines 221 to 226 in 2941f9d

    
           // flow-list selection exactly the way the j/k keys do (see onKey). 
        
           func (m Model) onRightPaneWheel(ev tea.MouseMsg) (tea.Model, tea.Cmd) { 
        
           	if m.viewing { 
        
           		var cmd tea.Cmd 
        
           		m.vp, cmd = m.vp.Update(ev) 
        
           		return m, cmd

-leader=ctrl+i and -leader=ctrl+m silently break Tab and Enter. ParseLeader accepts the whole ctrl+a…ctrl+z range, but KeyCtrlI == KeyTab and KeyCtrlM == KeyEnter in bubbletea (identical ASCII control codes 0x09/0x0D). With -leader=ctrl+m every Enter is swallowed as the leader — the child can't submit a newline and traffic-pane Enter stops opening detail; ctrl+i does the same to Tab. Rejecting those two specs in ParseLeader would fix it.

cli-capture/internal/tui/leader.go

Lines 46 to 49 in 2941f9d

    
           if letter, ok := strings.CutPrefix(s, "ctrl+"); ok && len(letter) == 1 { 
        
           	if c := letter[0]; c >= 'a' && c <= 'z' { 
        
           		key := tea.KeyCtrlA + tea.KeyType(c-'a') 
        
           		return Leader{

Lower priority: VT.Close() blocks on <-v.done, but pumpReplies can be parked in resp.Write(target.Pty) rather than in term.Read, and term.Close() only unblocks a pending Read. If the target has stopped draining its stdin when you quit, the pty buffer fills, that write blocks, and shutdown hangs with no timeout. Narrow (needs a live-but-stalled child at exit), but it's new surface this PR adds.

cli-capture/internal/terminal/vt.go

Lines 86 to 90 in 2941f9d

    
           func (v *VT) Close() error { 
        
           	err := v.term.Close() 
        
           	<-v.done 
        
           	return err 
        
           }

The emulator swap itself checks out: verified against the x/vt source that the unlocked Read matches upstream's own SafeEmulator concurrency contract, and that blank cells render fine (EmptyCell is not the zero Cell, so columns don't collapse). The session.go 0600 and ca.go 0700 tightening are good hardening.

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

citizen-123 · 2026-07-30T11:15:29Z

Thanks for this — the truecolor fix and the mouse work are both really nice, and the disclosure about the x/vt race was exactly the right call.

Heads-up that main has moved a fair bit since you branched (the config-driven keymap, resizable panes, repeater, and CI all landed), so this needed more than a rebase — the keymap and pane-geometry changes overlap directly with your commits. Rather than send it back to you for another round, I carried your work forward onto the new main myself and opened it as a separate PR: #17.

What I reconciled, so nothing here is lost:

Kept the x/vt swap, mouse support, JSON detail expansion, and the file-mode/security fixes.
Dropped the -leader flag and leader.go — main now makes the leader configurable through its keymap config, so I wired your runtime mouse-capture toggle in as a first-class keymap action (ActMouseCapture, leader m) instead.
Re-based the mouse hit-testing on main's splitRatio geometry, and carried your pane-overflow fix onto it — turns out main reintroduced the same +2 overflow via the resize feature, so your fix is still needed (and now covered by a test that measures the actual rendered row width).

Your commits are preserved with authorship intact; #17 is the one I'll drive to merge. The x/vt race is documented there too, with options for getting CI green. Since #17 supersedes this, I'd suggest closing this PR once you've had a chance to look — but I'll leave that to you.

Generated by Claude Code

cameronsjo · 2026-07-30T22:23:09Z

Superseded by #17, which carries these twelve commits forward onto current main with authorship preserved — and which now also closes the review findings that were still live in them. This branch is a long way behind main and conflicting; there is nothing here that #17 does not have.

Closing in favour of it. Thanks for doing the carry-forward rather than sending this back for another rebase.

cameronsjo added 12 commits July 26, 2026 13:06

citizen-123 self-requested a review July 27, 2026 23:18

citizen-123 mentioned this pull request Jul 30, 2026

Truecolor terminal backend (x/vt) + mouse support, rebased onto config/keymap + resizable panes #17

Open

cameronsjo closed this Jul 30, 2026

cameronsjo deleted the feat/mouse-vt-migration-and-fixes branch July 30, 2026 22:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Swap vt10x for charmbracelet/x/vt, add mouse support to the split-pane TUI - #3

Swap vt10x for charmbracelet/x/vt, add mouse support to the split-pane TUI#3
cameronsjo wants to merge 12 commits into
citizen-123:mainfrom
cameronsjo:feat/mouse-vt-migration-and-fixes

cameronsjo commented Jul 27, 2026

Uh oh!

citizen-123 commented Jul 27, 2026

Uh oh!

citizen-123 commented Jul 30, 2026

Uh oh!

cameronsjo commented Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

cameronsjo commented Jul 27, 2026

What's here

Disclosure: a data race in charmbracelet/x/vt

Notes

Uh oh!

citizen-123 commented Jul 27, 2026

Code review

Uh oh!

citizen-123 commented Jul 30, 2026

Uh oh!

cameronsjo commented Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Disclosure: a data race in `charmbracelet/x/vt`