fix(copy): ignore stale pointer resets

commit: d9ee20f74218e35c0f971ce9479350149e41c675
author: brice · 3mo ago · 2026-07-15 08:49 UTC
committer: brice · 3mo ago · 2026-07-15 09:45 UTC
parent: b26b9f7
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

2 files · +103 -15

M internal/usecase/daemon/input.go +19 -15


@@ -311,6 +311,19 @@ func snapshotCopyMouseInput(rt *overlayRuntime) (copyMouseInputSnapshot, bool) {
311	311		}, true
312	312		}
313	313
	314	+	// invalidateSnapshotCopyPointer clears only the pointer captured with the
	315	+	// input snapshot. Mapping runs outside copyMu, so a newer press can otherwise
	316	+	// be erased by an older rejected event.
	317	+	func invalidateSnapshotCopyPointer(rt *overlayRuntime, snapshot copyMouseInputSnapshot) {
	318	+	rt.copyMu.Lock()
	319	+	defer rt.copyMu.Unlock()
	320	+	if rt.copyPointerEpoch != snapshot.epoch \|\| rt.copyMode != snapshot.mode \|\|
	321	+	rt.copyPane != snapshot.pane \|\| rt.copyDocument != snapshot.document {
	322	+	return
	323	+	}
	324	+	rt.invalidateCopyPointerLocked()
	325	+	}
	326	+
314	327		// handleActiveCopyMouse keeps every drag tied to the pane/document captured at
315	328		// press time. Geometry is resolved after copyMu is released to preserve the
316	329		// daemon's copyMu -> tab/pane lock prohibition.

@@ -320,19 +333,17 @@ func (d Daemon) handleActiveCopyMouse(sess session, ac attachedClient, tb ta
320	333		if !active {
321	334		return
322	335		}
	336	+	if d.beforeCopyMouseMap != nil {
	337	+	d.beforeCopyMouseMap()
	338	+	}
323	339		cfg := d.currentFloatingConfig()
324	340		// Once pressed, use the committed rectangle captured by that press. A
325	341		// later floating/layout change must not turn an in-flight drag into a hit
326	342		// on a different pane.
327	343		if ev.Type != mouse.Press && snapshot.pointer.valid && snapshot.pointer.pane == snapshot.pane && snapshot.pointer.document == snapshot.document {
328		-	if d.beforeCopyMouseMap != nil {
329		-	d.beforeCopyMouseMap()
330		-	}
331	344		mapped, ok := mapCopyMouse(ev, snapshot.pointer.geometry, snapshot.viewportTop, snapshot.document, ev.Type == mouse.Motion)
332	345		if ev.Type == mouse.Release && !ok {
333		-	rt.copyMu.Lock()
334		-	rt.invalidateCopyPointerLocked()
335		-	rt.copyMu.Unlock()
	346	+	invalidateSnapshotCopyPointer(rt, snapshot)
336	347		return
337	348		}
338	349		if ok {

@@ -353,16 +364,11 @@ func (d Daemon) handleActiveCopyMouse(sess session, ac attachedClient, tb ta
353	364		tb.mu.Unlock()
354	365		if !ok {
355	366		if ev.Type == mouse.Release \|\| ev.Type == mouse.Press {
356		-	rt.copyMu.Lock()
357		-	rt.invalidateCopyPointerLocked()
358		-	rt.copyMu.Unlock()
	367	+	invalidateSnapshotCopyPointer(rt, snapshot)
359	368		}
360	369		return
361	370		}
362	371		clamp := ev.Type == mouse.Motion \|\| ev.Type == mouse.Release
363		-	if d.beforeCopyMouseMap != nil {
364		-	d.beforeCopyMouseMap()
365		-	}
366	372		mapped, mappedOK := mapCopyMouse(ev, geometry, snapshot.viewportTop, snapshot.document, clamp)
367	373		if !mappedOK {
368	374		if ev.Type == mouse.Press {

@@ -381,9 +387,7 @@ func (d Daemon) handleActiveCopyMouse(sess session, ac attachedClient, tb ta
381	387		}
382	388		}
383	389		if ev.Type == mouse.Release \|\| ev.Type == mouse.Press {
384		-	rt.copyMu.Lock()
385		-	rt.invalidateCopyPointerLocked()
386		-	rt.copyMu.Unlock()
	390	+	invalidateSnapshotCopyPointer(rt, snapshot)
387	391		}
388	392		return
389	393		}

 	}, true
 }
+// invalidateSnapshotCopyPointer clears only the pointer captured with the
+// input snapshot. Mapping runs outside copyMu, so a newer press can otherwise
+// be erased by an older rejected event.
+func invalidateSnapshotCopyPointer(rt *overlayRuntime, snapshot copyMouseInputSnapshot) {
+	rt.copyMu.Lock()
+	defer rt.copyMu.Unlock()
+	if rt.copyPointerEpoch != snapshot.epoch || rt.copyMode != snapshot.mode ||
+		rt.copyPane != snapshot.pane || rt.copyDocument != snapshot.document {
+		return
+	}
+	rt.invalidateCopyPointerLocked()
+}
 // handleActiveCopyMouse keeps every drag tied to the pane/document captured at
 // press time. Geometry is resolved after copyMu is released to preserve the
 // daemon's copyMu -> tab/pane lock prohibition.
 	if !active {
 		return
 	}
+	if d.beforeCopyMouseMap != nil {
+		d.beforeCopyMouseMap()
+	}
 	cfg := d.currentFloatingConfig()
 	// Once pressed, use the committed rectangle captured by that press. A
 	// later floating/layout change must not turn an in-flight drag into a hit
 	// on a different pane.
 	if ev.Type != mouse.Press && snapshot.pointer.valid && snapshot.pointer.pane == snapshot.pane && snapshot.pointer.document == snapshot.document {
-		if d.beforeCopyMouseMap != nil {
-			d.beforeCopyMouseMap()
-		}
 		mapped, ok := mapCopyMouse(ev, snapshot.pointer.geometry, snapshot.viewportTop, snapshot.document, ev.Type == mouse.Motion)
 		if ev.Type == mouse.Release && !ok {
-			rt.copyMu.Lock()
+			invalidateSnapshotCopyPointer(rt, snapshot)
-			rt.invalidateCopyPointerLocked()
-			rt.copyMu.Unlock()
 			return
 		}
 		if ok {
 	tb.mu.Unlock()
 	if !ok {
 		if ev.Type == mouse.Release || ev.Type == mouse.Press {
-			rt.copyMu.Lock()
+			invalidateSnapshotCopyPointer(rt, snapshot)
-			rt.invalidateCopyPointerLocked()
-			rt.copyMu.Unlock()
 		}
 		return
 	}
 	clamp := ev.Type == mouse.Motion || ev.Type == mouse.Release
-	if d.beforeCopyMouseMap != nil {
-		d.beforeCopyMouseMap()
-	}
 	mapped, mappedOK := mapCopyMouse(ev, geometry, snapshot.viewportTop, snapshot.document, clamp)
 	if !mappedOK {
 		if ev.Type == mouse.Press {
 			}
 		}
 		if ev.Type == mouse.Release || ev.Type == mouse.Press {
-			rt.copyMu.Lock()
+			invalidateSnapshotCopyPointer(rt, snapshot)
-			rt.invalidateCopyPointerLocked()
-			rt.copyMu.Unlock()
 		}
 		return
 	}

M internal/usecase/daemon/input_test.go +84 -0


@@ -1473,6 +1473,90 @@ func TestMouseCollapsedStackBarExpandsAndFocuses(t *testing.T) {
1473	1473		require.Equal(t, layout.PaneID("pane-2"), tb.tree.Root.Expanded)
1474	1474		}
1475	1475
	1476	+	// TestActiveCopyMouseIgnoresStalePointerResets makes the mapping/reset race
	1477	+	// deterministic. Each path captures a copy-input snapshot, yields while that
	1478	+	// snapshot is outside copyMu, then starts a new pointer. The old event must
	1479	+	// never invalidate the new interaction.
	1480	+	func TestActiveCopyMouseIgnoresStalePointerResets(t *testing.T) {
	1481	+	for _, tc := range []struct {
	1482	+	name string
	1483	+	prepare func(t testing.T, d Daemon, sess session, ac attachedClient) mouse.Event
	1484	+	}{
	1485	+	{
	1486	+	name: "press-owned release rejects mapping",
	1487	+	prepare: func(t testing.T, d Daemon, sess session, ac attachedClient) mouse.Event {
	1488	+	t.Helper()
	1489	+	p := sess.activeTab().focusedPane()
	1490	+	tb := sess.activeTab()
	1491	+	tb.mu.Lock()
	1492	+	geometry, ok := hitTestCopyMouseGeometryLocked(tb, d.currentFloatingConfig(), 1, 2)
	1493	+	tb.mu.Unlock()
	1494	+	require.True(t, ok)
	1495	+	ac.overlays.copyMu.Lock()
	1496	+	ac.overlays.beginCopyPointerLocked(copyPointerState{pane: p, document: ac.overlays.copyDocument, geometry: geometry})
	1497	+	ac.overlays.copyMu.Unlock()
	1498	+	return mouse.Event{Button: mouse.Left, Type: mouse.Release, Col: geometry.content.X - 1, Row: geometry.content.Y}
	1499	+	},
	1500	+	},
	1501	+	{
	1502	+	name: "current geometry disappears",
	1503	+	prepare: func(t testing.T, _ Daemon, sess session, _ attachedClient) mouse.Event {
	1504	+	t.Helper()
	1505	+	tb := sess.activeTab()
	1506	+	tb.mu.Lock()
	1507	+	tb.tree = nil
	1508	+	tb.mu.Unlock()
	1509	+	return mouse.Event{Button: mouse.Left, Type: mouse.Press, Col: 1, Row: 2}
	1510	+	},
	1511	+	},
	1512	+	{
	1513	+	name: "fresh press rejects document mapping",
	1514	+	prepare: func(t testing.T, _ Daemon, sess session, _ attachedClient) mouse.Event {
	1515	+	t.Helper()
	1516	+	tb := sess.activeTab()
	1517	+	tb.mu.Lock()
	1518	+	tb.size.Rows = 40 // valid layout coordinate, outside the old document.
	1519	+	tb.mu.Unlock()
	1520	+	return mouse.Event{Button: mouse.Left, Type: mouse.Press, Col: 1, Row: 30}
	1521	+	},
	1522	+	},
	1523	+	} {
	1524	+	t.Run(tc.name, func(t *testing.T) {
	1525	+	p, release := newBlockingPTY(t)
	1526	+	defer release()
	1527	+	d, sess, ac, sends := newManualSessionWithPTYs(t, p)
	1528	+	d.enterCopyMode(sess, ac)
	1529	+	mustOutputData(t, sends)
	1530	+	ev := tc.prepare(t, d, sess, ac)
	1531	+
	1532	+	var want copyPointerState
	1533	+	d.beforeCopyMouseMap = func() {
	1534	+	ac.overlays.copyMu.Lock()
	1535	+	ac.overlays.beginCopyPointerLocked(copyPointerState{
	1536	+	pane: ac.overlays.copyPane,
	1537	+	document: ac.overlays.copyDocument,
	1538	+	press: scopy.Pos{Row: 7, Col: 7},
	1539	+	})
	1540	+	want = ac.overlays.copyPointer
	1541	+	ac.overlays.copyMu.Unlock()
	1542	+	}
	1543	+	defer func() { d.beforeCopyMouseMap = nil }()
	1544	+
	1545	+	d.handleActiveCopyMouse(sess, ac, sess.activeTab(), ev)
	1546	+
	1547	+	ac.overlays.copyMu.Lock()
	1548	+	defer ac.overlays.copyMu.Unlock()
	1549	+	require.True(t, want.valid, "test seam must start the newer pointer")
	1550	+	require.True(t, ac.overlays.copyPointer.valid)
	1551	+	require.Equal(t, want.epoch, ac.overlays.copyPointerEpoch)
	1552	+	require.Equal(t, want.epoch, ac.overlays.copyPointer.epoch)
	1553	+	require.Same(t, want.pane, ac.overlays.copyPointer.pane)
	1554	+	require.Same(t, want.document, ac.overlays.copyPointer.document)
	1555	+	require.Equal(t, want.press, ac.overlays.copyPointer.press)
	1556	+	})
	1557	+	}
	1558	+	}
	1559	+
1476	1560		func TestActiveCopyMouseRejectsViewportChangeAfterMappingSnapshot(t *testing.T) {
1477	1561		p, release := newBlockingPTY(t)
1478	1562		defer release()


@@ -1473,6 +1473,90 @@ func TestMouseCollapsedStackBarExpandsAndFocuses(t *testing.T) {
1473	require.Equal(t, layout.PaneID("pane-2"), tb.tree.Root.Expanded)	1473		require.Equal(t, layout.PaneID("pane-2"), tb.tree.Root.Expanded)
1474	}	1474		}
1475		1475
		1476	+	// TestActiveCopyMouseIgnoresStalePointerResets makes the mapping/reset race
		1477	+	// deterministic. Each path captures a copy-input snapshot, yields while that
		1478	+	// snapshot is outside copyMu, then starts a new pointer. The old event must
		1479	+	// never invalidate the new interaction.
		1480	+	func TestActiveCopyMouseIgnoresStalePointerResets(t *testing.T) {
		1481	+	for _, tc := range []struct {
		1482	+	name string
		1483	+	prepare func(t testing.T, d Daemon, sess session, ac attachedClient) mouse.Event
		1484	+	}{
		1485	+	{
		1486	+	name: "press-owned release rejects mapping",
		1487	+	prepare: func(t testing.T, d Daemon, sess session, ac attachedClient) mouse.Event {
		1488	+	t.Helper()
		1489	+	p := sess.activeTab().focusedPane()
		1490	+	tb := sess.activeTab()
		1491	+	tb.mu.Lock()
		1492	+	geometry, ok := hitTestCopyMouseGeometryLocked(tb, d.currentFloatingConfig(), 1, 2)
		1493	+	tb.mu.Unlock()
		1494	+	require.True(t, ok)
		1495	+	ac.overlays.copyMu.Lock()
		1496	+	ac.overlays.beginCopyPointerLocked(copyPointerState{pane: p, document: ac.overlays.copyDocument, geometry: geometry})
		1497	+	ac.overlays.copyMu.Unlock()
		1498	+	return mouse.Event{Button: mouse.Left, Type: mouse.Release, Col: geometry.content.X - 1, Row: geometry.content.Y}
		1499	+	},
		1500	+	},
		1501	+	{
		1502	+	name: "current geometry disappears",
		1503	+	prepare: func(t testing.T, _ Daemon, sess session, _ attachedClient) mouse.Event {
		1504	+	t.Helper()
		1505	+	tb := sess.activeTab()
		1506	+	tb.mu.Lock()
		1507	+	tb.tree = nil
		1508	+	tb.mu.Unlock()
		1509	+	return mouse.Event{Button: mouse.Left, Type: mouse.Press, Col: 1, Row: 2}
		1510	+	},
		1511	+	},
		1512	+	{
		1513	+	name: "fresh press rejects document mapping",
		1514	+	prepare: func(t testing.T, _ Daemon, sess session, _ attachedClient) mouse.Event {
		1515	+	t.Helper()
		1516	+	tb := sess.activeTab()
		1517	+	tb.mu.Lock()
		1518	+	tb.size.Rows = 40 // valid layout coordinate, outside the old document.
		1519	+	tb.mu.Unlock()
		1520	+	return mouse.Event{Button: mouse.Left, Type: mouse.Press, Col: 1, Row: 30}
		1521	+	},
		1522	+	},
		1523	+	} {
		1524	+	t.Run(tc.name, func(t *testing.T) {
		1525	+	p, release := newBlockingPTY(t)
		1526	+	defer release()
		1527	+	d, sess, ac, sends := newManualSessionWithPTYs(t, p)
		1528	+	d.enterCopyMode(sess, ac)
		1529	+	mustOutputData(t, sends)
		1530	+	ev := tc.prepare(t, d, sess, ac)
		1531	+
		1532	+	var want copyPointerState
		1533	+	d.beforeCopyMouseMap = func() {
		1534	+	ac.overlays.copyMu.Lock()
		1535	+	ac.overlays.beginCopyPointerLocked(copyPointerState{
		1536	+	pane: ac.overlays.copyPane,
		1537	+	document: ac.overlays.copyDocument,
		1538	+	press: scopy.Pos{Row: 7, Col: 7},
		1539	+	})
		1540	+	want = ac.overlays.copyPointer
		1541	+	ac.overlays.copyMu.Unlock()
		1542	+	}
		1543	+	defer func() { d.beforeCopyMouseMap = nil }()
		1544	+
		1545	+	d.handleActiveCopyMouse(sess, ac, sess.activeTab(), ev)
		1546	+
		1547	+	ac.overlays.copyMu.Lock()
		1548	+	defer ac.overlays.copyMu.Unlock()
		1549	+	require.True(t, want.valid, "test seam must start the newer pointer")
		1550	+	require.True(t, ac.overlays.copyPointer.valid)
		1551	+	require.Equal(t, want.epoch, ac.overlays.copyPointerEpoch)
		1552	+	require.Equal(t, want.epoch, ac.overlays.copyPointer.epoch)
		1553	+	require.Same(t, want.pane, ac.overlays.copyPointer.pane)
		1554	+	require.Same(t, want.document, ac.overlays.copyPointer.document)
		1555	+	require.Equal(t, want.press, ac.overlays.copyPointer.press)
		1556	+	})
		1557	+	}
		1558	+	}
		1559	+
1476	func TestActiveCopyMouseRejectsViewportChangeAfterMappingSnapshot(t *testing.T) {	1560		func TestActiveCopyMouseRejectsViewportChangeAfterMappingSnapshot(t *testing.T) {
1477	p, release := newBlockingPTY(t)	1561		p, release := newBlockingPTY(t)
1478	defer release()	1562		defer release()