e03e10d fix(copy): validate mode publication atomically · vev-vt

M internal/usecase/daemon/copymode.go +21 -11


@@ -45,6 +45,9 @@ func copyTargetPane(rt overlayRuntime) pane {
45	45		}
46	46		rt.copyMu.Lock()
47	47		defer rt.copyMu.Unlock()
	48	+	if rt.copyMode == nil {
	49	+	return nil
	50	+	}
48	51		return rt.copyPane
49	52		}
50	53

@@ -94,10 +97,10 @@ func (d Daemon) enterCopyMode(sess session, ac *attachedClient) {
94	97		d.paint(sess, ac, true)
95	98		}
96	99
97		-	// publishCopyMode publishes an immutable copy document, then validates the
98		-	// captured target without overlapping copyMu with session or tab locks. A tab
99		-	// transition or pane close may race capture; retaining copy state in either
100		-	// case would let an old document render or be yanked.
	100	+	// publishCopyMode installs a non-renderable candidate, validates its captured
	101	+	// target without overlapping copyMu with session or tab locks, then atomically
	102	+	// activates that same candidate. A tab transition, pane close, or newer
	103	+	// publication invalidates the candidate before it can render or be yanked.
101	104		func (d Daemon) publishCopyMode(sess session, ac attachedClient, tb tab, p pane, document scopy.Snapshot, prepare func(scopy.Mode)) bool {
102	105		if sess == nil \|\| ac == nil \|\| tb == nil \|\| p == nil {
103	106		return false

@@ -108,7 +111,10 @@ func (d Daemon) publishCopyMode(sess session, ac attachedClient, tb tab, p *
108	111		}
109	112		rt := ac.overlays
110	113		rt.copyMu.Lock()
111		-	rt.copyMode = mode
	114	+	d.stopCopyPendingTimerLocked(ac)
	115	+	rt.copyPending = nil
	116	+	rt.copyMode = nil
	117	+	rt.copyCandidate = mode
112	118		rt.copySnapshot = &document
113	119		rt.copyPane = p
114	120		rt.copySearch = nil

@@ -125,15 +131,19 @@ func (d Daemon) publishCopyMode(sess session, ac attachedClient, tb tab, p *
125	131		valid = tb.panes[p.id] == p \|\| (tb.floating.state == floatingVisible && tb.floating.pane == p)
126	132		tb.mu.Unlock()
127	133		}
128		-	if valid {
129		-	return true
130		-	}
	134	+
131	135		rt.copyMu.Lock()
132		-	if rt.copyMode == mode && rt.copyPane == p {
	136	+	defer rt.copyMu.Unlock()
	137	+	if rt.copyCandidate != mode \|\| rt.copyPane != p \|\| rt.copySnapshot != &document {
	138	+	return false
	139	+	}
	140	+	if !valid {
133	141		rt.clearCopyModeLocked()
	142	+	return false
134	143		}
135		-	rt.copyMu.Unlock()
136		-	return false
	144	+	rt.copyCandidate = nil
	145	+	rt.copyMode = mode
	146	+	return true
137	147		}
138	148
139	149		func (d Daemon) copyMouse(sess session, ac *attachedClient, ev mouse.Event) {


@@ -45,6 +45,9 @@ func copyTargetPane(rt overlayRuntime) pane {
45		}	45		}
46		rt.copyMu.Lock()	46		rt.copyMu.Lock()
47		defer rt.copyMu.Unlock()	47		defer rt.copyMu.Unlock()
			48	+	if rt.copyMode == nil {
			49	+	return nil
			50	+	}
48		return rt.copyPane	51		return rt.copyPane
49		}	52		}
50			53

@@ -94,10 +97,10 @@ func (d Daemon) enterCopyMode(sess session, ac *attachedClient) {
94		d.paint(sess, ac, true)	97		d.paint(sess, ac, true)
95		}	98		}
96			99
97	-	// publishCopyMode publishes an immutable copy document, then validates the	100	+	// publishCopyMode installs a non-renderable candidate, validates its captured
98	-	// captured target without overlapping copyMu with session or tab locks. A tab	101	+	// target without overlapping copyMu with session or tab locks, then atomically
99	-	// transition or pane close may race capture; retaining copy state in either	102	+	// activates that same candidate. A tab transition, pane close, or newer
100	-	// case would let an old document render or be yanked.	103	+	// publication invalidates the candidate before it can render or be yanked.
101		func (d Daemon) publishCopyMode(sess session, ac attachedClient, tb tab, p pane, document scopy.Snapshot, prepare func(scopy.Mode)) bool {	104		func (d Daemon) publishCopyMode(sess session, ac attachedClient, tb tab, p pane, document scopy.Snapshot, prepare func(scopy.Mode)) bool {
102		if sess == nil \|\| ac == nil \|\| tb == nil \|\| p == nil {	105		if sess == nil \|\| ac == nil \|\| tb == nil \|\| p == nil {
103		return false	106		return false

@@ -108,7 +111,10 @@ func (d Daemon) publishCopyMode(sess session, ac attachedClient, tb tab, p *
108		}	111		}
109		rt := ac.overlays	112		rt := ac.overlays
110		rt.copyMu.Lock()	113		rt.copyMu.Lock()
111	-	rt.copyMode = mode	114	+	d.stopCopyPendingTimerLocked(ac)
			115	+	rt.copyPending = nil
			116	+	rt.copyMode = nil
			117	+	rt.copyCandidate = mode
112		rt.copySnapshot = &document	118		rt.copySnapshot = &document
113		rt.copyPane = p	119		rt.copyPane = p
114		rt.copySearch = nil	120		rt.copySearch = nil

@@ -125,15 +131,19 @@ func (d Daemon) publishCopyMode(sess session, ac attachedClient, tb tab, p *
125		valid = tb.panes[p.id] == p \|\| (tb.floating.state == floatingVisible && tb.floating.pane == p)	131		valid = tb.panes[p.id] == p \|\| (tb.floating.state == floatingVisible && tb.floating.pane == p)
126		tb.mu.Unlock()	132		tb.mu.Unlock()
127		}	133		}
128	-	if valid {	134	+
129	-	return true
130	-	}
131		rt.copyMu.Lock()	135		rt.copyMu.Lock()
132	-	if rt.copyMode == mode && rt.copyPane == p {	136	+	defer rt.copyMu.Unlock()
			137	+	if rt.copyCandidate != mode \|\| rt.copyPane != p \|\| rt.copySnapshot != &document {
			138	+	return false
			139	+	}
			140	+	if !valid {
133		rt.clearCopyModeLocked()	141		rt.clearCopyModeLocked()
			142	+	return false
134		}	143		}
135	-	rt.copyMu.Unlock()	144	+	rt.copyCandidate = nil
136	-	return false	145	+	rt.copyMode = mode
			146	+	return true
137		}	147		}
138			148
139		func (d Daemon) copyMouse(sess session, ac *attachedClient, ev mouse.Event) {	149		func (d Daemon) copyMouse(sess session, ac *attachedClient, ev mouse.Event) {

M internal/usecase/daemon/copymode_lifecycle_test.go +45 -0


@@ -2,7 +2,9 @@ package daemon
2	2
3	3		import (
4	4		"testing"
	5	+	"time"
5	6
	7	+	"github.com/stretchr/testify/assert"
6	8		"github.com/stretchr/testify/require"
7	9
8	10		"github.com/bnema/vev/internal/domain"

@@ -82,6 +84,49 @@ func TestCopyModeLifecycleCloseOnlyPaneInTabClearsRecoveredClientState(t *testin
82	84		require.Nil(t, copyTargetPane(ac.overlays))
83	85		}
84	86
	87	+	func TestCopyModeLifecycleDoesNotRenderCandidateBeforeValidation(t *testing.T) {
	88	+	d, sess, ac, _, releases := newManualTabSession(t, 2)
	89	+	defer func() {
	90	+	for _, release := range releases {
	91	+	release()
	92	+	}
	93	+	}()
	94	+	tb := sess.activeTab()
	95	+	p := tb.focusedPane()
	96	+	p.mu.Lock()
	97	+	document := scopy.NewSnapshot(p.scrollback, p.screen.Frame)
	98	+	p.mu.Unlock()
	99	+
	100	+	// Hold session validation after publication and make the captured target
	101	+	// stale. A render snapshot taken in this window must not expose the
	102	+	// candidate document.
	103	+	sess.mu.Lock()
	104	+	sess.active = 1
	105	+	result := make(chan bool, 1)
	106	+	go func() {
	107	+	result <- d.publishCopyMode(sess, ac, tb, p, document, nil)
	108	+	}()
	109	+	published := assert.Eventually(t, func() bool {
	110	+	ac.overlays.copyMu.Lock()
	111	+	defer ac.overlays.copyMu.Unlock()
	112	+	return ac.overlays.copyPane == p && ac.overlays.copySnapshot != nil
	113	+	}, time.Second, time.Millisecond)
	114	+	if !published {
	115	+	sess.mu.Unlock()
	116	+	<-result
	117	+	return
	118	+	}
	119	+	renderSnapshot := ac.overlays.SnapshotForRender()
	120	+	candidateWasRenderable := renderSnapshot.copyActive
	121	+	renderSnapshot.Unlock()
	122	+	sess.mu.Unlock()
	123	+
	124	+	require.False(t, <-result)
	125	+	require.False(t, candidateWasRenderable)
	126	+	require.False(t, ac.overlays.copyActive())
	127	+	require.Nil(t, copyTargetPane(ac.overlays))
	128	+	}
	129	+
85	130		func TestCopyModeLifecycleRejectsPublicationForInactiveOrRemovedPane(t *testing.T) {
86	131		for _, tc := range []struct {
87	132		name string


@@ -2,7 +2,9 @@ package daemon
2		2
3	import (	3		import (
4	"testing"	4		"testing"
		5	+	"time"
5		6
		7	+	"github.com/stretchr/testify/assert"
6	"github.com/stretchr/testify/require"	8		"github.com/stretchr/testify/require"
7		9
8	"github.com/bnema/vev/internal/domain"	10		"github.com/bnema/vev/internal/domain"

@@ -82,6 +84,49 @@ func TestCopyModeLifecycleCloseOnlyPaneInTabClearsRecoveredClientState(t *testin
82	require.Nil(t, copyTargetPane(ac.overlays))	84		require.Nil(t, copyTargetPane(ac.overlays))
83	}	85		}
84		86
		87	+	func TestCopyModeLifecycleDoesNotRenderCandidateBeforeValidation(t *testing.T) {
		88	+	d, sess, ac, _, releases := newManualTabSession(t, 2)
		89	+	defer func() {
		90	+	for _, release := range releases {
		91	+	release()
		92	+	}
		93	+	}()
		94	+	tb := sess.activeTab()
		95	+	p := tb.focusedPane()
		96	+	p.mu.Lock()
		97	+	document := scopy.NewSnapshot(p.scrollback, p.screen.Frame)
		98	+	p.mu.Unlock()
		99	+
		100	+	// Hold session validation after publication and make the captured target
		101	+	// stale. A render snapshot taken in this window must not expose the
		102	+	// candidate document.
		103	+	sess.mu.Lock()
		104	+	sess.active = 1
		105	+	result := make(chan bool, 1)
		106	+	go func() {
		107	+	result <- d.publishCopyMode(sess, ac, tb, p, document, nil)
		108	+	}()
		109	+	published := assert.Eventually(t, func() bool {
		110	+	ac.overlays.copyMu.Lock()
		111	+	defer ac.overlays.copyMu.Unlock()
		112	+	return ac.overlays.copyPane == p && ac.overlays.copySnapshot != nil
		113	+	}, time.Second, time.Millisecond)
		114	+	if !published {
		115	+	sess.mu.Unlock()
		116	+	<-result
		117	+	return
		118	+	}
		119	+	renderSnapshot := ac.overlays.SnapshotForRender()
		120	+	candidateWasRenderable := renderSnapshot.copyActive
		121	+	renderSnapshot.Unlock()
		122	+	sess.mu.Unlock()
		123	+
		124	+	require.False(t, <-result)
		125	+	require.False(t, candidateWasRenderable)
		126	+	require.False(t, ac.overlays.copyActive())
		127	+	require.Nil(t, copyTargetPane(ac.overlays))
		128	+	}
		129	+
85	func TestCopyModeLifecycleRejectsPublicationForInactiveOrRemovedPane(t *testing.T) {	130		func TestCopyModeLifecycleRejectsPublicationForInactiveOrRemovedPane(t *testing.T) {
86	for _, tc := range []struct {	131		for _, tc := range []struct {
87	name string	132		name string

M internal/usecase/daemon/copymode_test.go +12 -2


@@ -546,7 +546,8 @@ func TestCopyModeLoneEscapeExitsAfterDelay(t *testing.T) {
546	546		}
547	547
548	548		func TestCopyModePendingEscapeDoesNotCloseNewMode(t *testing.T) {
549		-	p, _ := newBlockingPTY(t)
	549	+	p, release := newBlockingPTY(t)
	550	+	defer release()
550	551		d, sess, ac, sends := newManualSessionWithPTYs(t, p)
551	552		clk := &signalClock{timers: make(chan *signalTimer, 1)}
552	553		d.clock = clk

@@ -560,6 +561,14 @@ func TestCopyModePendingEscapeDoesNotCloseNewMode(t *testing.T) {
560	561		d.enterCopyMode(sess, ac)
561	562		awaitFrame(t, sends, ports.MsgOutput)
562	563
	564	+	ac.overlays.copyMu.Lock()
	565	+	require.Nil(t, ac.overlays.copyESC.timer)
	566	+	require.Nil(t, ac.overlays.copyESC.done)
	567	+	require.Empty(t, ac.overlays.copyPending)
	568	+	ac.overlays.copyMu.Unlock()
	569	+	d.handleInput(sess, ac, []byte("x"))
	570	+	require.True(t, ac.overlays.copyActive(), "pending input from the replaced mode affected the new mode")
	571	+
563	572		timer.ch <- time.Now()
564	573		require.Never(t, func() bool {
565	574		ac.overlays.copyMu.Lock()

@@ -609,7 +618,8 @@ func TestCopyModeEmptyYankDoesNotClearClipboard(t *testing.T) {
609	618		}
610	619
611	620		func TestHandleCopyInputUsesImmutableSnapshotWithoutPaneLock(t *testing.T) {
612		-	p, _ := newBlockingPTY(t)
	621	+	p, release := newBlockingPTY(t)
	622	+	defer release()
613	623		d, sess, ac, sends := newManualSessionWithPTYs(t, p)
614	624		pane := sess.activeTab().focusedPane()
615	625		pane.screen.Write([]byte("live"))

 }
 func TestCopyModePendingEscapeDoesNotCloseNewMode(t *testing.T) {
-	p, _ := newBlockingPTY(t)
+	p, release := newBlockingPTY(t)
+	defer release()
 	d, sess, ac, sends := newManualSessionWithPTYs(t, p)
 	clk := &signalClock{timers: make(chan *signalTimer, 1)}
 	d.clock = clk
 	d.enterCopyMode(sess, ac)
 	awaitFrame(t, sends, ports.MsgOutput)
+	ac.overlays.copyMu.Lock()
+	require.Nil(t, ac.overlays.copyESC.timer)
+	require.Nil(t, ac.overlays.copyESC.done)
+	require.Empty(t, ac.overlays.copyPending)
+	ac.overlays.copyMu.Unlock()
+	d.handleInput(sess, ac, []byte("x"))
+	require.True(t, ac.overlays.copyActive(), "pending input from the replaced mode affected the new mode")
 	timer.ch <- time.Now()
 	require.Never(t, func() bool {
 		ac.overlays.copyMu.Lock()
 }
 func TestHandleCopyInputUsesImmutableSnapshotWithoutPaneLock(t *testing.T) {
-	p, _ := newBlockingPTY(t)
+	p, release := newBlockingPTY(t)
+	defer release()
 	d, sess, ac, sends := newManualSessionWithPTYs(t, p)
 	pane := sess.activeTab().focusedPane()
 	pane.screen.Write([]byte("live"))

M internal/usecase/daemon/overlay_runtime.go +3 -1


@@ -30,6 +30,7 @@ type overlayRuntime struct {
30	30
31	31		copyMu sync.Mutex
32	32		copyMode *scopy.Mode
	33	+	copyCandidate *scopy.Mode
33	34		copySnapshot *scopy.Snapshot
34	35		copyPane *pane
35	36		copyPending []byte

@@ -103,6 +104,7 @@ func (rt *overlayRuntime) copySearchActive() bool {
103	104
104	105		func (rt *overlayRuntime) clearCopyModeLocked() {
105	106		rt.copyMode = nil
	107	+	rt.copyCandidate = nil
106	108		rt.copySnapshot = nil
107	109		rt.copyPane = nil
108	110		rt.copySearch = nil

@@ -115,7 +117,7 @@ func (rt overlayRuntime) clearCopyModeForPane(p pane) bool {
115	117		}
116	118		rt.copyMu.Lock()
117	119		defer rt.copyMu.Unlock()
118		-	if rt.copyMode == nil \|\| rt.copyPane != p {
	120	+	if rt.copyPane != p \|\| (rt.copyMode == nil && rt.copyCandidate == nil) {
119	121		return false
120	122		}
121	123		rt.clearCopyModeLocked()

 	copyMu                sync.Mutex
 	copyMode              *scopy.Mode
+	copyCandidate         *scopy.Mode
 	copySnapshot          *scopy.Snapshot
 	copyPane              *pane
 	copyPending           []byte
 func (rt *overlayRuntime) clearCopyModeLocked() {
 	rt.copyMode = nil
+	rt.copyCandidate = nil
 	rt.copySnapshot = nil
 	rt.copyPane = nil
 	rt.copySearch = nil
 	}
 	rt.copyMu.Lock()
 	defer rt.copyMu.Unlock()
-	if rt.copyMode == nil || rt.copyPane != p {
+	if rt.copyPane != p || (rt.copyMode == nil && rt.copyCandidate == nil) {
 		return false
 	}
 	rt.clearCopyModeLocked()