a84c06c fix(client): address bracketed paste review findings · vev

A internal/ports/paste.go +9 -0

@@ -0,0 +1,9 @@
	1	+	package ports
	2	+
	3	+	// Bracketed-paste markers bracket terminal paste payloads when bracketed-paste
	4	+	// mode is enabled. The client coalescer and daemon key router both depend on
	5	+	// these bytes staying in sync.
	6	+	var (
	7	+	BracketedPasteOpenMarker = []byte("\x1b[200~")
	8	+	BracketedPasteCloseMarker = []byte("\x1b[201~")
	9	+	)

@@ -0,0 +1,9 @@
	1	+	package ports
	2	+
	3	+	// Bracketed-paste markers bracket terminal paste payloads when bracketed-paste
	4	+	// mode is enabled. The client coalescer and daemon key router both depend on
	5	+	// these bytes staying in sync.
	6	+	var (
	7	+	BracketedPasteOpenMarker = []byte("\x1b[200~")
	8	+	BracketedPasteCloseMarker = []byte("\x1b[201~")
	9	+	)

M internal/usecase/client/client_test.go +85 -0


@@ -107,6 +107,31 @@ func (r *oneShotBlockingReader) Read(p []byte) (int, error) {
107	107
108	108		func (r *oneShotBlockingReader) unblock() { close(r.done) }
109	109
	110	+	type chunkedBlockingReader struct {
	111	+	chunks [][]byte
	112	+	done chan struct{}
	113	+	mu sync.Mutex
	114	+	next int
	115	+	}
	116	+
	117	+	func newChunkedBlockingReader(chunks ...[]byte) *chunkedBlockingReader {
	118	+	return &chunkedBlockingReader{chunks: chunks, done: make(chan struct{})}
	119	+	}
	120	+
	121	+	func (r *chunkedBlockingReader) Read(p []byte) (int, error) {
	122	+	r.mu.Lock()
	123	+	defer r.mu.Unlock()
	124	+	if r.next < len(r.chunks) {
	125	+	chunk := r.chunks[r.next]
	126	+	r.next++
	127	+	return copy(p, chunk), nil
	128	+	}
	129	+	<-r.done
	130	+	return 0, io.EOF
	131	+	}
	132	+
	133	+	func (r *chunkedBlockingReader) unblock() { close(r.done) }
	134	+
110	135		func newHappyTerminal(t testing.T, out bytes.Buffer, restoreCount atomic.Int32, resizeCh chan domain.Size) (portsmocks.MockTerminal, *blockingReader) {
111	136		tm := portsmocks.NewMockTerminal(t)
112	137		tm.EXPECT().Size().Return(domain.Size{Cols: 80, Rows: 24}, nil).Once()

@@ -486,6 +511,66 @@ func TestAttachStdinForwardsSGRMouseReportAsSingleFrame(t *testing.T) {
486	511		}
487	512		}
488	513
	514	+	func TestAttachStdinCoalescesSplitBracketedPaste(t *testing.T) {
	515	+	var out bytes.Buffer
	516	+	var restoreCount atomic.Int32
	517	+	resizeCh := make(chan domain.Size)
	518	+	paste := []byte("\x1b[200~hello\nworld\x1b[201~")
	519	+	input := newChunkedBlockingReader([]byte("\x1b[200~hello\n"), []byte("world\x1b[201~"))
	520	+	defer input.unblock()
	521	+
	522	+	tm := portsmocks.NewMockTerminal(t)
	523	+	tm.EXPECT().Size().Return(domain.Size{Cols: 80, Rows: 24}, nil).Once()
	524	+	tm.EXPECT().EnterRaw().Return(func() error { restoreCount.Add(1); return nil }, nil).Once()
	525	+	tm.EXPECT().In().Return(input).Maybe()
	526	+	tm.EXPECT().Out().Return(&out).Maybe()
	527	+	tm.EXPECT().Flush().Return(nil).Maybe()
	528	+	tm.EXPECT().ResizeEvents().Return(resizeCh).Maybe()
	529	+
	530	+	gotInput := make(chan []byte, 1)
	531	+	allowDetach := make(chan struct{})
	532	+	tr := portsmocks.NewMockTransport(t)
	533	+	tr.EXPECT().Send(isType(ports.MsgHello)).Return(nil).Once()
	534	+	tr.EXPECT().Send(isType(ports.MsgInput)).RunAndReturn(func(f ports.Frame) error {
	535	+	in, err := ports.UnmarshalInput(f.Payload)
	536	+	require.NoError(t, err)
	537	+	gotInput <- in.Data
	538	+	close(allowDetach)
	539	+	return nil
	540	+	}).Once()
	541	+	welcome := frameOf(ports.MsgWelcome, ports.MarshalWelcome(ports.Welcome{SessionID: "s1"}))
	542	+	detached := frameOf(ports.MsgDetached, ports.MarshalDetached(ports.Detached{Reason: ports.ReasonDetach}))
	543	+	recvCh := make(chan recvItem, 1)
	544	+	recvCh <- recvItem{f: welcome}
	545	+	closed := make(chan struct{})
	546	+	tr.EXPECT().Recv().RunAndReturn(func() (ports.Frame, error) {
	547	+	select {
	548	+	case it := <-recvCh:
	549	+	return it.f, it.err
	550	+	case <-allowDetach:
	551	+	select {
	552	+	case <-closed:
	553	+	return ports.Frame{}, io.EOF
	554	+	default:
	555	+	close(closed)
	556	+	return detached, nil
	557	+	}
	558	+	case <-closed:
	559	+	return ports.Frame{}, io.EOF
	560	+	}
	561	+	}).Maybe()
	562	+	tr.EXPECT().Close().Return(nil).Once()
	563	+
	564	+	err := client.Attach(context.Background(), tr, tm, realClock{}, ports.IntentEphemeral, "")
	565	+	require.NoError(t, err)
	566	+	select {
	567	+	case got := <-gotInput:
	568	+	require.Equal(t, paste, got, "split bracketed paste must arrive as one intact MsgInput frame")
	569	+	case <-time.After(2 * time.Second):
	570	+	t.Fatal("split bracketed paste input was not sent")
	571	+	}
	572	+	}
	573	+
489	574		func TestAttachForwardsResize(t *testing.T) {
490	575		var out bytes.Buffer
491	576		var restoreCount atomic.Int32


@@ -107,6 +107,31 @@ func (r *oneShotBlockingReader) Read(p []byte) (int, error) {
107		107
108	func (r *oneShotBlockingReader) unblock() { close(r.done) }	108		func (r *oneShotBlockingReader) unblock() { close(r.done) }
109		109
		110	+	type chunkedBlockingReader struct {
		111	+	chunks [][]byte
		112	+	done chan struct{}
		113	+	mu sync.Mutex
		114	+	next int
		115	+	}
		116	+
		117	+	func newChunkedBlockingReader(chunks ...[]byte) *chunkedBlockingReader {
		118	+	return &chunkedBlockingReader{chunks: chunks, done: make(chan struct{})}
		119	+	}
		120	+
		121	+	func (r *chunkedBlockingReader) Read(p []byte) (int, error) {
		122	+	r.mu.Lock()
		123	+	defer r.mu.Unlock()
		124	+	if r.next < len(r.chunks) {
		125	+	chunk := r.chunks[r.next]
		126	+	r.next++
		127	+	return copy(p, chunk), nil
		128	+	}
		129	+	<-r.done
		130	+	return 0, io.EOF
		131	+	}
		132	+
		133	+	func (r *chunkedBlockingReader) unblock() { close(r.done) }
		134	+
110	func newHappyTerminal(t testing.T, out bytes.Buffer, restoreCount atomic.Int32, resizeCh chan domain.Size) (portsmocks.MockTerminal, *blockingReader) {	135		func newHappyTerminal(t testing.T, out bytes.Buffer, restoreCount atomic.Int32, resizeCh chan domain.Size) (portsmocks.MockTerminal, *blockingReader) {
111	tm := portsmocks.NewMockTerminal(t)	136		tm := portsmocks.NewMockTerminal(t)
112	tm.EXPECT().Size().Return(domain.Size{Cols: 80, Rows: 24}, nil).Once()	137		tm.EXPECT().Size().Return(domain.Size{Cols: 80, Rows: 24}, nil).Once()

@@ -486,6 +511,66 @@ func TestAttachStdinForwardsSGRMouseReportAsSingleFrame(t *testing.T) {
486	}	511		}
487	}	512		}
488		513
		514	+	func TestAttachStdinCoalescesSplitBracketedPaste(t *testing.T) {
		515	+	var out bytes.Buffer
		516	+	var restoreCount atomic.Int32
		517	+	resizeCh := make(chan domain.Size)
		518	+	paste := []byte("\x1b[200~hello\nworld\x1b[201~")
		519	+	input := newChunkedBlockingReader([]byte("\x1b[200~hello\n"), []byte("world\x1b[201~"))
		520	+	defer input.unblock()
		521	+
		522	+	tm := portsmocks.NewMockTerminal(t)
		523	+	tm.EXPECT().Size().Return(domain.Size{Cols: 80, Rows: 24}, nil).Once()
		524	+	tm.EXPECT().EnterRaw().Return(func() error { restoreCount.Add(1); return nil }, nil).Once()
		525	+	tm.EXPECT().In().Return(input).Maybe()
		526	+	tm.EXPECT().Out().Return(&out).Maybe()
		527	+	tm.EXPECT().Flush().Return(nil).Maybe()
		528	+	tm.EXPECT().ResizeEvents().Return(resizeCh).Maybe()
		529	+
		530	+	gotInput := make(chan []byte, 1)
		531	+	allowDetach := make(chan struct{})
		532	+	tr := portsmocks.NewMockTransport(t)
		533	+	tr.EXPECT().Send(isType(ports.MsgHello)).Return(nil).Once()
		534	+	tr.EXPECT().Send(isType(ports.MsgInput)).RunAndReturn(func(f ports.Frame) error {
		535	+	in, err := ports.UnmarshalInput(f.Payload)
		536	+	require.NoError(t, err)
		537	+	gotInput <- in.Data
		538	+	close(allowDetach)
		539	+	return nil
		540	+	}).Once()
		541	+	welcome := frameOf(ports.MsgWelcome, ports.MarshalWelcome(ports.Welcome{SessionID: "s1"}))
		542	+	detached := frameOf(ports.MsgDetached, ports.MarshalDetached(ports.Detached{Reason: ports.ReasonDetach}))
		543	+	recvCh := make(chan recvItem, 1)
		544	+	recvCh <- recvItem{f: welcome}
		545	+	closed := make(chan struct{})
		546	+	tr.EXPECT().Recv().RunAndReturn(func() (ports.Frame, error) {
		547	+	select {
		548	+	case it := <-recvCh:
		549	+	return it.f, it.err
		550	+	case <-allowDetach:
		551	+	select {
		552	+	case <-closed:
		553	+	return ports.Frame{}, io.EOF
		554	+	default:
		555	+	close(closed)
		556	+	return detached, nil
		557	+	}
		558	+	case <-closed:
		559	+	return ports.Frame{}, io.EOF
		560	+	}
		561	+	}).Maybe()
		562	+	tr.EXPECT().Close().Return(nil).Once()
		563	+
		564	+	err := client.Attach(context.Background(), tr, tm, realClock{}, ports.IntentEphemeral, "")
		565	+	require.NoError(t, err)
		566	+	select {
		567	+	case got := <-gotInput:
		568	+	require.Equal(t, paste, got, "split bracketed paste must arrive as one intact MsgInput frame")
		569	+	case <-time.After(2 * time.Second):
		570	+	t.Fatal("split bracketed paste input was not sent")
		571	+	}
		572	+	}
		573	+
489	func TestAttachForwardsResize(t *testing.T) {	574		func TestAttachForwardsResize(t *testing.T) {
490	var out bytes.Buffer	575		var out bytes.Buffer
491	var restoreCount atomic.Int32	576		var restoreCount atomic.Int32

M internal/usecase/client/paste_coalescer.go +17 -17


@@ -8,14 +8,6 @@ import (
8	8		"github.com/bnema/vev/internal/ports"
9	9		)
10	10
11		-	// Bracketed-paste markers. A terminal in bracketed-paste mode brackets pasted
12		-	// text with these so the receiving application can tell a paste from typed
13		-	// input.
14		-	var (
15		-	pasteOpenMarker = []byte("\x1b[200~")
16		-	pasteCloseMarker = []byte("\x1b[201~")
17		-	)
18		-
19	11		const (
20	12		// pasteFlushDelay bounds how long a trailing strict prefix of the opening
21	13		// marker is held before it is emitted as ordinary bytes. It only has to

@@ -91,9 +83,17 @@ func (c *pasteCoalescer) Scan(data []byte) {
91	83		func (c *pasteCoalescer) process(data []byte) {
92	84		for {
93	85		if c.buffering {
	86	+	searchFrom := len(c.buf)
	87	+	if overlap := len(ports.BracketedPasteCloseMarker) - 1; searchFrom > overlap {
	88	+	searchFrom -= overlap
	89	+	} else {
	90	+	searchFrom = 0
	91	+	}
94	92		c.buf = append(c.buf, data...)
95		-	data = nil
96		-	idx := bytes.Index(c.buf, pasteCloseMarker)
	93	+	idx := bytes.Index(c.buf[searchFrom:], ports.BracketedPasteCloseMarker)
	94	+	if idx >= 0 {
	95	+	idx += searchFrom
	96	+	}
97	97		if idx < 0 {
98	98		if len(c.buf) > maxPasteBuffer {
99	99		c.emit(c.buf)

@@ -101,7 +101,7 @@ func (c *pasteCoalescer) process(data []byte) {
101	101		}
102	102		return
103	103		}
104		-	end := idx + len(pasteCloseMarker)
	104	+	end := idx + len(ports.BracketedPasteCloseMarker)
105	105		c.emit(c.buf[:end])
106	106		rest := append([]byte(nil), c.buf[end:]...)
107	107		c.resetBuffering()

@@ -113,7 +113,7 @@ func (c *pasteCoalescer) process(data []byte) {
113	113		return
114	114		}
115	115
116		-	if idx := bytes.Index(data, pasteOpenMarker); idx >= 0 {
	116	+	if idx := bytes.Index(data, ports.BracketedPasteOpenMarker); idx >= 0 {
117	117		if idx > 0 {
118	118		c.emit(data[:idx])
119	119		}

@@ -240,12 +240,12 @@ func (c *pasteCoalescer) stopSafetyTimer() {
240	240		// is a non-empty strict prefix of the opening marker (e.g. a trailing "\x1b" or
241	241		// "\x1b[20"). A full marker match is handled by the caller before this runs.
242	242		func trailingOpenPrefixLen(data []byte) int {
243		-	max := len(pasteOpenMarker) - 1
244		-	if max > len(data) {
245		-	max = len(data)
	243	+	matchLimit := len(ports.BracketedPasteOpenMarker) - 1
	244	+	if matchLimit > len(data) {
	245	+	matchLimit = len(data)
246	246		}
247		-	for n := max; n >= 1; n-- {
248		-	if bytes.Equal(data[len(data)-n:], pasteOpenMarker[:n]) {
	247	+	for n := matchLimit; n >= 1; n-- {
	248	+	if bytes.Equal(data[len(data)-n:], ports.BracketedPasteOpenMarker[:n]) {
249	249		return n
250	250		}
251	251		}

 	"github.com/bnema/vev/internal/ports"
 )
-// Bracketed-paste markers. A terminal in bracketed-paste mode brackets pasted
-// text with these so the receiving application can tell a paste from typed
-// input.
-var (
-	pasteOpenMarker  = []byte("\x1b[200~")
-	pasteCloseMarker = []byte("\x1b[201~")
-)
 const (
 	// pasteFlushDelay bounds how long a trailing strict prefix of the opening
 	// marker is held before it is emitted as ordinary bytes. It only has to
 func (c *pasteCoalescer) process(data []byte) {
 	for {
 		if c.buffering {
+			searchFrom := len(c.buf)
+			if overlap := len(ports.BracketedPasteCloseMarker) - 1; searchFrom > overlap {
+				searchFrom -= overlap
+			} else {
+				searchFrom = 0
+			}
 			c.buf = append(c.buf, data...)
-			data = nil
+			idx := bytes.Index(c.buf[searchFrom:], ports.BracketedPasteCloseMarker)
-			idx := bytes.Index(c.buf, pasteCloseMarker)
+			if idx >= 0 {
+				idx += searchFrom
+			}
 			if idx < 0 {
 				if len(c.buf) > maxPasteBuffer {
 					c.emit(c.buf)
 				}
 				return
 			}
-			end := idx + len(pasteCloseMarker)
+			end := idx + len(ports.BracketedPasteCloseMarker)
 			c.emit(c.buf[:end])
 			rest := append([]byte(nil), c.buf[end:]...)
 			c.resetBuffering()
 			return
 		}
-		if idx := bytes.Index(data, pasteOpenMarker); idx >= 0 {
+		if idx := bytes.Index(data, ports.BracketedPasteOpenMarker); idx >= 0 {
 			if idx > 0 {
 				c.emit(data[:idx])
 			}
 // is a non-empty strict prefix of the opening marker (e.g. a trailing "\x1b" or
 // "\x1b[20"). A full marker match is handled by the caller before this runs.
 func trailingOpenPrefixLen(data []byte) int {
-	max := len(pasteOpenMarker) - 1
+	matchLimit := len(ports.BracketedPasteOpenMarker) - 1
-	if max > len(data) {
+	if matchLimit > len(data) {
-		max = len(data)
+		matchLimit = len(data)
 	}
-	for n := max; n >= 1; n-- {
+	for n := matchLimit; n >= 1; n-- {
-		if bytes.Equal(data[len(data)-n:], pasteOpenMarker[:n]) {
+		if bytes.Equal(data[len(data)-n:], ports.BracketedPasteOpenMarker[:n]) {
 			return n
 		}
 	}

M internal/usecase/client/paste_coalescer_test.go +12 -0


@@ -117,6 +117,18 @@ func TestPasteCoalescerByteByByte(t *testing.T) {
117	117		"paste fed one byte at a time must still emit as one contiguous paste")
118	118		}
119	119
	120	+	func TestPasteCoalescerBackToBackPastesInOneRead(t *testing.T) {
	121	+	pc, _, col := newTestCoalescer()
	122	+	defer pc.Close()
	123	+
	124	+	first := []byte("\x1b[200~first\x1b[201~")
	125	+	second := []byte("\x1b[200~second\x1b[201~")
	126	+	pc.Scan(append(append([]byte(nil), first...), second...))
	127	+
	128	+	require.Equal(t, [][]byte{first, second}, col.snapshot(),
	129	+	"back-to-back pastes in one read must emit as two intact paste frames")
	130	+	}
	131	+
120	132		func TestPasteCoalescerLoneEscapeFlushedAfterTimer(t *testing.T) {
121	133		pc, clk, col := newTestCoalescer()
122	134		defer pc.Close()


@@ -117,6 +117,18 @@ func TestPasteCoalescerByteByByte(t *testing.T) {
117	"paste fed one byte at a time must still emit as one contiguous paste")	117		"paste fed one byte at a time must still emit as one contiguous paste")
118	}	118		}
119		119
		120	+	func TestPasteCoalescerBackToBackPastesInOneRead(t *testing.T) {
		121	+	pc, _, col := newTestCoalescer()
		122	+	defer pc.Close()
		123	+
		124	+	first := []byte("\x1b[200~first\x1b[201~")
		125	+	second := []byte("\x1b[200~second\x1b[201~")
		126	+	pc.Scan(append(append([]byte(nil), first...), second...))
		127	+
		128	+	require.Equal(t, [][]byte{first, second}, col.snapshot(),
		129	+	"back-to-back pastes in one read must emit as two intact paste frames")
		130	+	}
		131	+
120	func TestPasteCoalescerLoneEscapeFlushedAfterTimer(t *testing.T) {	132		func TestPasteCoalescerLoneEscapeFlushedAfterTimer(t *testing.T) {
121	pc, clk, col := newTestCoalescer()	133		pc, clk, col := newTestCoalescer()
122	defer pc.Close()	134		defer pc.Close()

M internal/usecase/keys/router.go +3 -10


@@ -17,13 +17,6 @@ const (
17	17		ESCDelay = 40 * time.Millisecond
18	18		)
19	19
20		-	// Bracketed-paste markers. A paste whose opening and closing markers land in
21		-	// one frame is forwarded verbatim so its content can never trip a key binding.
22		-	var (
23		-	pasteOpenMarker = []byte("\x1b[200~")
24		-	pasteCloseMarker = []byte("\x1b[201~")
25		-	)
26		-
27	20		// Action is an intercepted vev key binding.
28	21		type Action int
29	22

@@ -112,7 +105,7 @@ func (r *Router) route(data []byte) {
112	105		return
113	106		}
114	107		remaining := data[i+1:]
115		-	if bytes.HasPrefix(data[i:], pasteOpenMarker) {
	108	+	if bytes.HasPrefix(data[i:], ports.BracketedPasteOpenMarker) {
116	109		// A bracketed paste whose closing marker is in this same frame is
117	110		// forwarded verbatim: pasted content bytes (including embedded
118	111		// ESC+letter Alt lookalikes or ESC [ 1 ; 3 A sequences) must never

@@ -120,8 +113,8 @@ func (r *Router) route(data []byte) {
120	113		// fall through to today's per-byte routing (the '[' passes through
121	114		// as a control prefix); Part A keeps pastes single-frame in
122	115		// practice, so no cross-frame paste state is tracked here.
123		-	if rel := bytes.Index(data[i:], pasteCloseMarker); rel >= 0 {
124		-	end := i + rel + len(pasteCloseMarker)
	116	+	if rel := bytes.Index(data[i:], ports.BracketedPasteCloseMarker); rel >= 0 {
	117	+	end := i + rel + len(ports.BracketedPasteCloseMarker)
125	118		buf = append(buf, data[i:end]...)
126	119		i = end
127	120		continue

 	ESCDelay      = 40 * time.Millisecond
 )
-// Bracketed-paste markers. A paste whose opening and closing markers land in
-// one frame is forwarded verbatim so its content can never trip a key binding.
-var (
-	pasteOpenMarker  = []byte("\x1b[200~")
-	pasteCloseMarker = []byte("\x1b[201~")
-)
 // Action is an intercepted vev key binding.
 type Action int
 			return
 		}
 		remaining := data[i+1:]
-		if bytes.HasPrefix(data[i:], pasteOpenMarker) {
+		if bytes.HasPrefix(data[i:], ports.BracketedPasteOpenMarker) {
 			// A bracketed paste whose closing marker is in this same frame is
 			// forwarded verbatim: pasted content bytes (including embedded
 			// ESC+letter Alt lookalikes or ESC [ 1 ; 3 A sequences) must never
 			// fall through to today's per-byte routing (the '[' passes through
 			// as a control prefix); Part A keeps pastes single-frame in
 			// practice, so no cross-frame paste state is tracked here.
-			if rel := bytes.Index(data[i:], pasteCloseMarker); rel >= 0 {
+			if rel := bytes.Index(data[i:], ports.BracketedPasteCloseMarker); rel >= 0 {
-				end := i + rel + len(pasteCloseMarker)
+				end := i + rel + len(ports.BracketedPasteCloseMarker)
 				buf = append(buf, data[i:end]...)
 				i = end
 				continue

M internal/usecase/keys/router_test.go +1 -1


@@ -506,7 +506,7 @@ func TestRouterForwardsSingleFramePasteVerbatim(t *testing.T) {
506	506		h := &captureHandler{}
507	507		r := NewRouter(clk, h)
508	508
509		-	paste := append(append(append([]byte(nil), pasteOpenMarker...), pasteBindingLookalikes...), pasteCloseMarker...)
	509	+	paste := append(append(append([]byte(nil), ports.BracketedPasteOpenMarker...), pasteBindingLookalikes...), ports.BracketedPasteCloseMarker...)
510	510		r.Route(paste)
511	511
512	512		require.Empty(t, h.actions, "paste content must not fire any binding")


@@ -506,7 +506,7 @@ func TestRouterForwardsSingleFramePasteVerbatim(t *testing.T) {
506		h := &captureHandler{}	506		h := &captureHandler{}
507		r := NewRouter(clk, h)	507		r := NewRouter(clk, h)
508			508
509	-	paste := append(append(append([]byte(nil), pasteOpenMarker...), pasteBindingLookalikes...), pasteCloseMarker...)	509	+	paste := append(append(append([]byte(nil), ports.BracketedPasteOpenMarker...), pasteBindingLookalikes...), ports.BracketedPasteCloseMarker...)
510		r.Route(paste)	510		r.Route(paste)
511			511
512		require.Empty(t, h.actions, "paste content must not fire any binding")	512		require.Empty(t, h.actions, "paste content must not fire any binding")