fix(tui): keep copy label background under "copied" flash - #3797
Conversation
The transient "copied" confirmation now derives the background in effect at the label position from the rendered line (SGR parsing) instead of hard-coding the code-block band, so user-message copy labels keep their background band too. Assisted-By: Claude <noreply@anthropic.com>
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🔴 CRITICAL
1 high-severity finding in new code.
| var bg color.Color | ||
| var state byte | ||
| for w := 0; line != "" && w <= col; { | ||
| seq, width, n, newState := ansi.DecodeSequence(line, state, p) |
There was a problem hiding this comment.
[high] backgroundAt can spin in an infinite loop when ansi.DecodeSequence returns n == 0
ansi.DecodeSequence can return n == 0 when the first byte of the remaining input is an invalid UTF-8 sequence (e.g., an isolated continuation byte 0x80–0xBF). In that case:
line = line[n:]is a no-op —linedoes not shrinkline != ""remainstruewnever advances (the invalid byte has visualwidth == 0)- The loop condition
w <= colstays satisfied
This causes a busy infinite loop, freezing the TUI.
From charmbracelet/x/ansi@v0.11.7/parser_decode.go, the "Invalid UTF-8 sequence" return path is:
// Invalid UTF-8 sequence
return b[:i], 0, i, NormalState // i == 0 on first byte → n == 0This path is reachable whenever a rendered line contains an isolated continuation byte or other invalid UTF-8. While charmbracelet's own rendering is unlikely to emit such bytes, backgroundAt is called on arbitrary terminal lines, so robustness matters.
Fix: add an early-out guard immediately after the DecodeSequence call:
| seq, width, n, newState := ansi.DecodeSequence(line, state, p) | |
| seq, width, n, newState := ansi.DecodeSequence(line, state, p) | |
| if n == 0 { | |
| break | |
| } |
| Confidence | Score |
|---|---|
| 🟡 moderate | 57/100 |
docker-agent
left a comment
There was a problem hiding this comment.
When a user clicks a copy button, the label briefly swaps to a "copied" confirmation. For code blocks, the background of that flash was already forced to match the code block's band. For user-message copy labels, which sit on the
BackgroundAltband, no such fix existed, so the transient label rendered with a transparent background and punched a visible hole in the colored band.The fix introduces
backgroundAt(line, col), which walks the ANSI SGR escape sequences of a rendered line and returns the background color in effect at the given column — handling truecolor, 256-indexed, basic, and bright colors, plus resets. The "copied" flash now calls this helper for all copy labels, replacing the code-block-only special case with a single, general path.New regression tests cover the user-message band case, the code-block band case, and a table-driven suite for
backgroundAtitself.