fix(clipboard): serialize osc52 forwarding

commit: fb1b2688073a79d875c1228411c7dc06a0ef8c83
author: brice · 3mo ago · 2026-07-04 16:30 UTC
parent: 65ad319
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

4 files · +136 -13

M internal/usecase/daemon/clipboard.go +56 -7


@@ -4,6 +4,11 @@ import (
4	4		scopy "github.com/bnema/vev/internal/usecase/copy"
5	5		)
6	6
	7	+	type clipboardForward struct {
	8	+	ac *attachedClient
	9	+	seq []byte
	10	+	}
	11	+
7	12		// forwardClipboardAsync re-emits an app-originated OSC 52 clipboard set
8	13		// request (already captured off the pane's screen while ptyReader held pane.mu,
9	14		// then handed here after that lock is released) to the session's attached

@@ -11,8 +16,8 @@ import (
11	16		// silently, matching copy mode's own cap (scopy.OSC52MaxPayloadBytes). The
12	17		// caller must not hold pane.mu, tab.mu, or session.mu.
13	18		//
14		-	// The bounded client send runs in its own goroutine so a slow or wedged client
15		-	// cannot make the PTY reader stop draining child output.
	19	+	// Requests are queued on the session and drained by one worker so clipboard
	20	+	// writes keep arrival order without making the PTY reader wait on client I/O.
16	21		func (d Daemon) forwardClipboardAsync(sess session, b64 string) {
17	22		seq := scopy.OSC52FromBase64(b64)
18	23		if seq == nil {

@@ -20,16 +25,60 @@ func (d Daemon) forwardClipboardAsync(sess session, b64 string) {
20	25		}
21	26
22	27		sess.mu.Lock()
	28	+	if sess.ctx != nil {
	29	+	select {
	30	+	case <-sess.ctx.Done():
	31	+	sess.mu.Unlock()
	32	+	return
	33	+	default:
	34	+	}
	35	+	}
23	36		ac := sess.client
24		-	sess.mu.Unlock()
25	37		if ac == nil {
	38	+	sess.mu.Unlock()
26	39		return
27	40		}
	41	+	sess.clipboardQueue = append(sess.clipboardQueue, clipboardForward{ac: ac, seq: seq})
	42	+	if !sess.clipboardWorkerRunning {
	43	+	sess.clipboardWorkerRunning = true
	44	+	go d.clipboardWorker(sess)
	45	+	}
	46	+	sess.mu.Unlock()
	47	+	}
28	48
29		-	go func() {
30		-	failed, err := d.boundedSendOutputErrTransport(ac, seq)
	49	+	func (d Daemon) clipboardWorker(sess session) {
	50	+	for {
	51	+	item, ok := nextClipboardForward(sess)
	52	+	if !ok {
	53	+	return
	54	+	}
	55	+	failed, err := d.boundedSendOutputErrTransport(item.ac, item.seq)
31	56		if err != nil {
32		-	d.detachOnSendError(sess, ac, failed)
	57	+	d.detachOnSendError(sess, item.ac, failed)
	58	+	}
	59	+	}
	60	+	}
	61	+
	62	+	func nextClipboardForward(sess *session) (clipboardForward, bool) {
	63	+	sess.mu.Lock()
	64	+	defer sess.mu.Unlock()
	65	+	if sess.ctx != nil {
	66	+	select {
	67	+	case <-sess.ctx.Done():
	68	+	sess.clipboardQueue = nil
	69	+	sess.clipboardWorkerRunning = false
	70	+	return clipboardForward{}, false
	71	+	default:
33	72		}
34		-	}()
	73	+	}
	74	+	if len(sess.clipboardQueue) == 0 {
	75	+	sess.clipboardWorkerRunning = false
	76	+	return clipboardForward{}, false
	77	+	}
	78	+	item := sess.clipboardQueue[0]
	79	+	copy(sess.clipboardQueue, sess.clipboardQueue[1:])
	80	+	var zero clipboardForward
	81	+	sess.clipboardQueue[len(sess.clipboardQueue)-1] = zero
	82	+	sess.clipboardQueue = sess.clipboardQueue[:len(sess.clipboardQueue)-1]
	83	+	return item, true
35	84		}


@@ -4,6 +4,11 @@ import (
4		scopy "github.com/bnema/vev/internal/usecase/copy"	4		scopy "github.com/bnema/vev/internal/usecase/copy"
5		)	5		)
6			6
			7	+	type clipboardForward struct {
			8	+	ac *attachedClient
			9	+	seq []byte
			10	+	}
			11	+
7		// forwardClipboardAsync re-emits an app-originated OSC 52 clipboard set	12		// forwardClipboardAsync re-emits an app-originated OSC 52 clipboard set
8		// request (already captured off the pane's screen while ptyReader held pane.mu,	13		// request (already captured off the pane's screen while ptyReader held pane.mu,
9		// then handed here after that lock is released) to the session's attached	14		// then handed here after that lock is released) to the session's attached

@@ -11,8 +16,8 @@ import (
11		// silently, matching copy mode's own cap (scopy.OSC52MaxPayloadBytes). The	16		// silently, matching copy mode's own cap (scopy.OSC52MaxPayloadBytes). The
12		// caller must not hold pane.mu, tab.mu, or session.mu.	17		// caller must not hold pane.mu, tab.mu, or session.mu.
13		//	18		//
14	-	// The bounded client send runs in its own goroutine so a slow or wedged client	19	+	// Requests are queued on the session and drained by one worker so clipboard
15	-	// cannot make the PTY reader stop draining child output.	20	+	// writes keep arrival order without making the PTY reader wait on client I/O.
16		func (d Daemon) forwardClipboardAsync(sess session, b64 string) {	21		func (d Daemon) forwardClipboardAsync(sess session, b64 string) {
17		seq := scopy.OSC52FromBase64(b64)	22		seq := scopy.OSC52FromBase64(b64)
18		if seq == nil {	23		if seq == nil {

@@ -20,16 +25,60 @@ func (d Daemon) forwardClipboardAsync(sess session, b64 string) {
20		}	25		}
21			26
22		sess.mu.Lock()	27		sess.mu.Lock()
			28	+	if sess.ctx != nil {
			29	+	select {
			30	+	case <-sess.ctx.Done():
			31	+	sess.mu.Unlock()
			32	+	return
			33	+	default:
			34	+	}
			35	+	}
23		ac := sess.client	36		ac := sess.client
24	-	sess.mu.Unlock()
25		if ac == nil {	37		if ac == nil {
			38	+	sess.mu.Unlock()
26		return	39		return
27		}	40		}
			41	+	sess.clipboardQueue = append(sess.clipboardQueue, clipboardForward{ac: ac, seq: seq})
			42	+	if !sess.clipboardWorkerRunning {
			43	+	sess.clipboardWorkerRunning = true
			44	+	go d.clipboardWorker(sess)
			45	+	}
			46	+	sess.mu.Unlock()
			47	+	}
28			48
29	-	go func() {	49	+	func (d Daemon) clipboardWorker(sess session) {
30	-	failed, err := d.boundedSendOutputErrTransport(ac, seq)	50	+	for {
			51	+	item, ok := nextClipboardForward(sess)
			52	+	if !ok {
			53	+	return
			54	+	}
			55	+	failed, err := d.boundedSendOutputErrTransport(item.ac, item.seq)
31		if err != nil {	56		if err != nil {
32	-	d.detachOnSendError(sess, ac, failed)	57	+	d.detachOnSendError(sess, item.ac, failed)
			58	+	}
			59	+	}
			60	+	}
			61	+
			62	+	func nextClipboardForward(sess *session) (clipboardForward, bool) {
			63	+	sess.mu.Lock()
			64	+	defer sess.mu.Unlock()
			65	+	if sess.ctx != nil {
			66	+	select {
			67	+	case <-sess.ctx.Done():
			68	+	sess.clipboardQueue = nil
			69	+	sess.clipboardWorkerRunning = false
			70	+	return clipboardForward{}, false
			71	+	default:
33		}	72		}
34	-	}()	73	+	}
			74	+	if len(sess.clipboardQueue) == 0 {
			75	+	sess.clipboardWorkerRunning = false
			76	+	return clipboardForward{}, false
			77	+	}
			78	+	item := sess.clipboardQueue[0]
			79	+	copy(sess.clipboardQueue, sess.clipboardQueue[1:])
			80	+	var zero clipboardForward
			81	+	sess.clipboardQueue[len(sess.clipboardQueue)-1] = zero
			82	+	sess.clipboardQueue = sess.clipboardQueue[:len(sess.clipboardQueue)-1]
			83	+	return item, true
35		}	84		}

M internal/usecase/daemon/clipboard_test.go +68 -0


@@ -6,6 +6,7 @@ import (
6	6		"io"
7	7		"strings"
8	8		"testing"
	9	+	"time"
9	10
10	11		"github.com/stretchr/testify/mock"
11	12		"github.com/stretchr/testify/require"

@@ -124,3 +125,70 @@ func TestPTYReaderClipboardNoAttachedClientDoesNotPanic(t *testing.T) {
124	125		d.ptyReader(sess, win, win.focusedPane())
125	126		})
126	127		}
	128	+
	129	+	func TestForwardClipboardAsyncSerializesClipboardWrites(t *testing.T) {
	130	+	d := newTestDaemon(t, nil, stubClock{})
	131	+	tr := newBlockingClipboardTransport()
	132	+	ac := &attachedClient{tr: tr, rend: renderer.New(renderer.Capabilities{})}
	133	+	sctx, cancel := context.WithCancel(context.Background())
	134	+	defer cancel()
	135	+	sess := &session{id: "clip-order", name: "clip-order", ctx: sctx, cancel: cancel, client: ac}
	136	+	ac.setSession(sess)
	137	+
	138	+	first := base64.StdEncoding.EncodeToString([]byte("first"))
	139	+	second := base64.StdEncoding.EncodeToString([]byte("second"))
	140	+	d.forwardClipboardAsync(sess, first)
	141	+	require.Equal(t, "first", <-tr.started)
	142	+
	143	+	d.forwardClipboardAsync(sess, second)
	144	+	select {
	145	+	case got := <-tr.started:
	146	+	t.Fatalf("second clipboard send started before first completed: %q", got)
	147	+	case <-time.After(50 * time.Millisecond):
	148	+	}
	149	+
	150	+	close(tr.releaseFirst)
	151	+	require.Equal(t, "first", <-tr.sent)
	152	+	require.Equal(t, "second", <-tr.started)
	153	+	require.Equal(t, "second", <-tr.sent)
	154	+	}
	155	+
	156	+	type blockingClipboardTransport struct {
	157	+	started chan string
	158	+	sent chan string
	159	+	releaseFirst chan struct{}
	160	+	}
	161	+
	162	+	func newBlockingClipboardTransport() *blockingClipboardTransport {
	163	+	return &blockingClipboardTransport{
	164	+	started: make(chan string, 2),
	165	+	sent: make(chan string, 2),
	166	+	releaseFirst: make(chan struct{}),
	167	+	}
	168	+	}
	169	+
	170	+	func (tr *blockingClipboardTransport) Send(f ports.Frame) error {
	171	+	out, err := ports.UnmarshalOutput(f.Payload)
	172	+	if err != nil {
	173	+	return err
	174	+	}
	175	+	data := string(out.Data)
	176	+	var label string
	177	+	switch {
	178	+	case strings.Contains(data, base64.StdEncoding.EncodeToString([]byte("first"))):
	179	+	label = "first"
	180	+	case strings.Contains(data, base64.StdEncoding.EncodeToString([]byte("second"))):
	181	+	label = "second"
	182	+	default:
	183	+	label = data
	184	+	}
	185	+	tr.started <- label
	186	+	if label == "first" {
	187	+	<-tr.releaseFirst
	188	+	}
	189	+	tr.sent <- label
	190	+	return nil
	191	+	}
	192	+
	193	+	func (tr *blockingClipboardTransport) Recv() (ports.Frame, error) { return ports.Frame{}, io.EOF }
	194	+	func (tr *blockingClipboardTransport) Close() error { return nil }


@@ -6,6 +6,7 @@ import (
6	"io"	6		"io"
7	"strings"	7		"strings"
8	"testing"	8		"testing"
		9	+	"time"
9		10
10	"github.com/stretchr/testify/mock"	11		"github.com/stretchr/testify/mock"
11	"github.com/stretchr/testify/require"	12		"github.com/stretchr/testify/require"

@@ -124,3 +125,70 @@ func TestPTYReaderClipboardNoAttachedClientDoesNotPanic(t *testing.T) {
124	d.ptyReader(sess, win, win.focusedPane())	125		d.ptyReader(sess, win, win.focusedPane())
125	})	126		})
126	}	127		}
		128	+
		129	+	func TestForwardClipboardAsyncSerializesClipboardWrites(t *testing.T) {
		130	+	d := newTestDaemon(t, nil, stubClock{})
		131	+	tr := newBlockingClipboardTransport()
		132	+	ac := &attachedClient{tr: tr, rend: renderer.New(renderer.Capabilities{})}
		133	+	sctx, cancel := context.WithCancel(context.Background())
		134	+	defer cancel()
		135	+	sess := &session{id: "clip-order", name: "clip-order", ctx: sctx, cancel: cancel, client: ac}
		136	+	ac.setSession(sess)
		137	+
		138	+	first := base64.StdEncoding.EncodeToString([]byte("first"))
		139	+	second := base64.StdEncoding.EncodeToString([]byte("second"))
		140	+	d.forwardClipboardAsync(sess, first)
		141	+	require.Equal(t, "first", <-tr.started)
		142	+
		143	+	d.forwardClipboardAsync(sess, second)
		144	+	select {
		145	+	case got := <-tr.started:
		146	+	t.Fatalf("second clipboard send started before first completed: %q", got)
		147	+	case <-time.After(50 * time.Millisecond):
		148	+	}
		149	+
		150	+	close(tr.releaseFirst)
		151	+	require.Equal(t, "first", <-tr.sent)
		152	+	require.Equal(t, "second", <-tr.started)
		153	+	require.Equal(t, "second", <-tr.sent)
		154	+	}
		155	+
		156	+	type blockingClipboardTransport struct {
		157	+	started chan string
		158	+	sent chan string
		159	+	releaseFirst chan struct{}
		160	+	}
		161	+
		162	+	func newBlockingClipboardTransport() *blockingClipboardTransport {
		163	+	return &blockingClipboardTransport{
		164	+	started: make(chan string, 2),
		165	+	sent: make(chan string, 2),
		166	+	releaseFirst: make(chan struct{}),
		167	+	}
		168	+	}
		169	+
		170	+	func (tr *blockingClipboardTransport) Send(f ports.Frame) error {
		171	+	out, err := ports.UnmarshalOutput(f.Payload)
		172	+	if err != nil {
		173	+	return err
		174	+	}
		175	+	data := string(out.Data)
		176	+	var label string
		177	+	switch {
		178	+	case strings.Contains(data, base64.StdEncoding.EncodeToString([]byte("first"))):
		179	+	label = "first"
		180	+	case strings.Contains(data, base64.StdEncoding.EncodeToString([]byte("second"))):
		181	+	label = "second"
		182	+	default:
		183	+	label = data
		184	+	}
		185	+	tr.started <- label
		186	+	if label == "first" {
		187	+	<-tr.releaseFirst
		188	+	}
		189	+	tr.sent <- label
		190	+	return nil
		191	+	}
		192	+
		193	+	func (tr *blockingClipboardTransport) Recv() (ports.Frame, error) { return ports.Frame{}, io.EOF }
		194	+	func (tr *blockingClipboardTransport) Close() error { return nil }

M internal/usecase/daemon/session.go +8 -6


@@ -47,12 +47,14 @@ type session struct {
47	47		ctx context.Context
48	48		cancel context.CancelFunc
49	49
50		-	mu sync.Mutex // guards tabs, active, and client
51		-	tabs []*tab
52		-	active int
53		-	client *attachedClient
54		-	cwd string
55		-	createdAt int64
	50	+	mu sync.Mutex // guards tabs, active, client, and clipboard queue state
	51	+	tabs []*tab
	52	+	active int
	53	+	client *attachedClient
	54	+	clipboardQueue []clipboardForward
	55	+	clipboardWorkerRunning bool
	56	+	cwd string
	57	+	createdAt int64
56	58		}
57	59
58	60		// tab is a pane layout container; pane owns PTY/screen/scrollback/render scheduling state.


@@ -47,12 +47,14 @@ type session struct {
47		ctx context.Context	47		ctx context.Context
48		cancel context.CancelFunc	48		cancel context.CancelFunc
49			49
50	-	mu sync.Mutex // guards tabs, active, and client	50	+	mu sync.Mutex // guards tabs, active, client, and clipboard queue state
51	-	tabs []*tab	51	+	tabs []*tab
52	-	active int	52	+	active int
53	-	client *attachedClient	53	+	client *attachedClient
54	-	cwd string	54	+	clipboardQueue []clipboardForward
55	-	createdAt int64	55	+	clipboardWorkerRunning bool
			56	+	cwd string
			57	+	createdAt int64
56		}	58		}
57			59
58		// tab is a pane layout container; pane owns PTY/screen/scrollback/render scheduling state.	60		// tab is a pane layout container; pane owns PTY/screen/scrollback/render scheduling state.

M pkg/vt/screen.go +4 -0


@@ -25,6 +25,10 @@ type screenState struct {
25	25		savedCursor cursorState
26	26		}
27	27
	28	+	// maxEscapeBufferLen must stay large enough for OSC 52 clipboard payloads
	29	+	// forwarded by internal/usecase/copy.OSC52MaxPayloadBytes after base64
	30	+	// expansion plus the OSC wrapper. pkg/vt cannot import internal/usecase/copy,
	31	+	// so keep this value in sync if that payload cap changes.
28	32		const maxEscapeBufferLen = 128 * 1024
29	33
30	34		type Screen struct {


@@ -25,6 +25,10 @@ type screenState struct {
25	savedCursor cursorState	25		savedCursor cursorState
26	}	26		}
27		27
		28	+	// maxEscapeBufferLen must stay large enough for OSC 52 clipboard payloads
		29	+	// forwarded by internal/usecase/copy.OSC52MaxPayloadBytes after base64
		30	+	// expansion plus the OSC wrapper. pkg/vt cannot import internal/usecase/copy,
		31	+	// so keep this value in sync if that payload cap changes.
28	const maxEscapeBufferLen = 128 * 1024	32		const maxEscapeBufferLen = 128 * 1024
29		33
30	type Screen struct {	34		type Screen struct {