#7 fix: address pre-release review findings since v0.9.0

state: merged · d5d679d · fast-forward
branch: fix/release-review → main
opened: bnema · 3d ago
updated: 3d ago

Pre-release review of v0.9.0..353b4e3 (drag and drop, output/command decoupling, focus flash, hint mode). Three reviewers, 12 findings, all fixed.

Drag and drop

Pre-v3 targets get drop without calling accept (feedback-only before v3).
Destroying the target's last data device clears the target; release then cancels the source instead of reporting a performed drop.
The negotiated action is sent to every target offer; destroying the accepting offer resets negotiation.
Client events that change hit testing (input region, map, resize…) re-route an active drag with a still pointer.
Negative test assertions use protocol barriers instead of fixed sleeps.

Commands and stats

Slot/stash placement tokens are read on the first toplevel map, independent of command draining. The SlotsPending command and its flush path are removed.
Ring and outbox stats sample current occupancy, so a long stall keeps being reported.

Hints and focus flash

Hint targets keep their source window geometry; results and clicks are rejected after a move or resize.
AT-SPI fallback walk stays within its node budget before allocating.
tesseract --list-langs has its own 5 s timeout.
A running focus flash stops when its rect changes.
Docs: accessibility is enabled on the first hint scan.

Performance

No new allocations on hot paths. Zero-allocation tests cover the drag re-hit and flash revalidation; /proc is read once per new toplevel.

Checks

make check, make race, make mocks-check, staticcheck ./...: pass.
Headless e2e: column slot and stash <cmd> placement (stash_index 1), focus flash on column focus and stop on column resize, hint labels on two OCR links and Escape cancel.
Not covered end to end: real client drag and drop (no scriptable DnD client), AT-SPI apps, multi-output.

Commit	Subject	Author	Date	Signature
d5d679d	Merge branch 'main' into fix/release-review	brice	3d ago	GPG Verified
4cf5ef0	Merge branch 'fix/review-outbox' into fix/release-review	brice	3d ago	GPG Verified
9a6eff8	refactor(core): remove SlotsPending command	brice	3d ago	GPG Verified
48fe6ad	Merge branch 'fix/review-hints' into fix/release-review	brice	3d ago	GPG Verified
91f7d4a	Merge branch 'fix/review-dnd' into fix/release-review	brice	3d ago	GPG Verified
638a799	fix(hints): reject stale geometry and bound scan discovery	brice	3d ago	GPG Verified
ef7b51c	fix(dnd): keep target lifetime and negotiation consistent	brice	3d ago	GPG Verified
cbbf1ee	fix(stats): sample stalled command queue occupancy each period	brice	3d ago	GPG Verified
d2670da	fix(wayland): read placement tokens independently of command drain	brice	3d ago	GPG Verified

Checks 2

34 files · +781 -331

2/2

M .mockery.yml +5 -0

show this file

M docs/config.md +2 -1

show this file

M internal/adapters/atspi/source.go +4 -5

show this file

M internal/adapters/atspi/source_test.go +40 -0

show this file

A internal/adapters/hints/language_runner_mock_test.go +130 -0

show this file

M internal/adapters/hints/ocr.go +21 -1

show this file

M internal/adapters/hints/ocr_test.go +34 -0

show this file

M internal/adapters/wayland/commands_test.go +2 -2

show this file

M internal/adapters/wayland/dnd.go +27 -7

show this file

M internal/adapters/wayland/dnd_test.go +137 -31

show this file

M internal/adapters/wayland/imagecapture_test.go +1 -30

show this file

M internal/adapters/wayland/keyboard_test.go +59 -23

show this file

M internal/adapters/wayland/postring.go +1 -0

show this file

M internal/adapters/wayland/postring_test.go +34 -0

show this file

M internal/adapters/wayland/server.go +0 -3

show this file

M internal/adapters/wayland/slot.go +4 -5

show this file

M internal/adapters/wayland/slot_test.go +3 -10

show this file

M internal/adapters/wayland/xdg.go +6 -1

show this file

M internal/core/core.go +13 -53

show this file

M internal/core/datadrag.go +15 -0

show this file

A internal/core/datadrag_alloc_test.go +34 -0

show this file

M internal/core/datadrag_test.go +12 -0

show this file

M internal/core/flash.go +35 -33

show this file

M internal/core/flash_test.go +47 -0

show this file

M internal/core/hint.go +26 -16

show this file

M internal/core/hint_test.go +46 -0


@@ -832,3 +832,49 @@ func TestHintDisabledNotesOnce(t *testing.T) {
832	832		t.Fatalf("disabled note ran %d times, want 1", got)
833	833		}
834	834		}
	835	+
	836	+	// Resizing window 1 leaves the old target centre inside it. Identity alone
	837	+	// therefore cannot protect either admission or the eventual synthetic click.
	838	+	func TestHintRejectsResizedSource(t *testing.T) {
	839	+	for _, accessibility := range []bool{false, true} {
	840	+	for _, beforeResult := range []bool{false, true} {
	841	+	t.Run(fmt.Sprintf("accessibility=%v/beforeResult=%v", accessibility, beforeResult), func(t *testing.T) {
	842	+	h := newHint(t, core.Options{})
	843	+	tg := h.inside(0)
	844	+	tg.Text = !accessibility
	845	+	if accessibility {
	846	+	tg.Window = 1
	847	+	}
	848	+	req := h.start()
	849	+	if !beforeResult {
	850	+	h.hints <- ports.HintScanResult{Seq: req.Seq, Output: req.Output, Targets: []ports.HintTarget{tg}}
	851	+	h.labelled(1, 0)
	852	+	}
	853	+	h.client <- ports.WindowMapped{ID: 2}
	854	+	s := h.until(func(s ports.Scene) bool { return len(s.Windows) == 2 })
	855	+	var resized ports.Rect
	856	+	for _, w := range s.Windows {
	857	+	if w.ID == 1 {
	858	+	resized = w.Rect
	859	+	}
	860	+	}
	861	+	x, y := tg.Rect.X+tg.Rect.W/2, tg.Rect.Y+tg.Rect.H/2
	862	+	if resized == h.win \|\| x < resized.X \|\| x >= resized.X+resized.W \|\| y < resized.Y \|\| y >= resized.Y+resized.H {
	863	+	t.Fatalf("setup: old target %+v not inside resized window %+v (was %+v)", tg.Rect, resized, h.win)
	864	+	}
	865	+	if beforeResult {
	866	+	h.hints <- ports.HintScanResult{Seq: req.Seq, Output: req.Output, Targets: []ports.HintTarget{tg}}
	867	+	h.labelled(0, 0)
	868	+	} else {
	869	+	h.drain()
	870	+	h.press("f", 0)
	871	+	for _, cmd := range h.barrier() {
	872	+	if _, ok := cmd.(ports.PointerButtonTo); ok {
	873	+	t.Fatal("clicked resized source window")
	874	+	}
	875	+	}
	876	+	}
	877	+	})
	878	+	}
	879	+	}
	880	+	}


@@ -832,3 +832,49 @@ func TestHintDisabledNotesOnce(t *testing.T) {
832	t.Fatalf("disabled note ran %d times, want 1", got)	832		t.Fatalf("disabled note ran %d times, want 1", got)
833	}	833		}
834	}	834		}
		835	+
		836	+	// Resizing window 1 leaves the old target centre inside it. Identity alone
		837	+	// therefore cannot protect either admission or the eventual synthetic click.
		838	+	func TestHintRejectsResizedSource(t *testing.T) {
		839	+	for _, accessibility := range []bool{false, true} {
		840	+	for _, beforeResult := range []bool{false, true} {
		841	+	t.Run(fmt.Sprintf("accessibility=%v/beforeResult=%v", accessibility, beforeResult), func(t *testing.T) {
		842	+	h := newHint(t, core.Options{})
		843	+	tg := h.inside(0)
		844	+	tg.Text = !accessibility
		845	+	if accessibility {
		846	+	tg.Window = 1
		847	+	}
		848	+	req := h.start()
		849	+	if !beforeResult {
		850	+	h.hints <- ports.HintScanResult{Seq: req.Seq, Output: req.Output, Targets: []ports.HintTarget{tg}}
		851	+	h.labelled(1, 0)
		852	+	}
		853	+	h.client <- ports.WindowMapped{ID: 2}
		854	+	s := h.until(func(s ports.Scene) bool { return len(s.Windows) == 2 })
		855	+	var resized ports.Rect
		856	+	for _, w := range s.Windows {
		857	+	if w.ID == 1 {
		858	+	resized = w.Rect
		859	+	}
		860	+	}
		861	+	x, y := tg.Rect.X+tg.Rect.W/2, tg.Rect.Y+tg.Rect.H/2
		862	+	if resized == h.win \|\| x < resized.X \|\| x >= resized.X+resized.W \|\| y < resized.Y \|\| y >= resized.Y+resized.H {
		863	+	t.Fatalf("setup: old target %+v not inside resized window %+v (was %+v)", tg.Rect, resized, h.win)
		864	+	}
		865	+	if beforeResult {
		866	+	h.hints <- ports.HintScanResult{Seq: req.Seq, Output: req.Output, Targets: []ports.HintTarget{tg}}
		867	+	h.labelled(0, 0)
		868	+	} else {
		869	+	h.drain()
		870	+	h.press("f", 0)
		871	+	for _, cmd := range h.barrier() {
		872	+	if _, ok := cmd.(ports.PointerButtonTo); ok {
		873	+	t.Fatal("clicked resized source window")
		874	+	}
		875	+	}
		876	+	}
		877	+	})
		878	+	}
		879	+	}
		880	+	}

M internal/core/outbox.go +3 -4


@@ -51,8 +51,7 @@ type CommandStats struct {
51	51		// MaxBacklog is the longest queue seen.
52	52		MaxBacklog uint64
53	53		// MaxQueueWait is the longest time a queued command took to reach the
54		-	// channel; MaxFlushWait the longest time core waited in flush (a spawn's
55		-	// SlotsPending, or a full outbox).
	54	+	// channel; MaxFlushWait the longest time core waited for outbox room.
56	55		MaxQueueWait, MaxFlushWait time.Duration
57	56		}
58	57

@@ -61,6 +60,7 @@ func (o *outbox) takeStats() CommandStats {
61	60		o.mu.Lock()
62	61		defer o.mu.Unlock()
63	62		s := o.stats
	63	+	s.MaxBacklog = max(s.MaxBacklog, o.pushed-o.sent)
64	64		o.stats = CommandStats{}
65	65		return s
66	66		}

@@ -171,8 +171,7 @@ func (o *outbox) push(out chan<- ports.ClientCommand, v ports.ClientCommand, sta
171	171		}
172	172
173	173		// flush waits until the entry seq is in out, or ctx ends. It is for the
174		-	// rare command another channel depends on (SlotsPending before a spawn);
175		-	// everything else never waits. Without a running sender (core driven
	174	+	// bounded outbox backpressure. Without a running sender (core driven
176	175		// without Run), it delivers the queue itself, in order.
177	176		func (o *outbox) flush(ctx context.Context, out chan<- ports.ClientCommand, seq uint64) error {
178	177		var start time.Time

 	// MaxBacklog is the longest queue seen.
 	MaxBacklog uint64
 	// MaxQueueWait is the longest time a queued command took to reach the
-	// channel; MaxFlushWait the longest time core waited in flush (a spawn's
+	// channel; MaxFlushWait the longest time core waited for outbox room.
-	// SlotsPending, or a full outbox).
 	MaxQueueWait, MaxFlushWait time.Duration
 }
 	o.mu.Lock()
 	defer o.mu.Unlock()
 	s := o.stats
+	s.MaxBacklog = max(s.MaxBacklog, o.pushed-o.sent)
 	o.stats = CommandStats{}
 	return s
 }
 }
 // flush waits until the entry seq is in out, or ctx ends. It is for the
-// rare command another channel depends on (SlotsPending before a spawn);
+// bounded outbox backpressure. Without a running sender (core driven
-// everything else never waits. Without a running sender (core driven
 // without Run), it delivers the queue itself, in order.
 func (o *outbox) flush(ctx context.Context, out chan<- ports.ClientCommand, seq uint64) error {
 	var start time.Time

M internal/core/outbox_test.go +20 -0


@@ -210,3 +210,23 @@ func TestOutboxQueueWaitIsPerEntry(t *testing.T) {
210	210		}
211	211		})
212	212		}
	213	+
	214	+	// A stalled sender must remain visible in every reporting period, including
	215	+	// when it has taken the queue into its private delivery batch.
	216	+	func TestOutboxStatsSampleOutstandingBacklog(t *testing.T) {
	217	+	o := newOutbox()
	218	+	out := make(chan ports.ClientCommand)
	219	+	for range outboxLimit {
	220	+	o.push(out, ports.FocusWindow{ID: 1}, ports.SecurityState{}, false)
	221	+	}
	222	+	batch := o.take()
	223	+	for range 2 {
	224	+	if got := o.takeStats().MaxBacklog; got != outboxLimit {
	225	+	t.Fatalf("backlog %d, want %d", got, outboxLimit)
	226	+	}
	227	+	}
	228	+	o.recycle(batch)
	229	+	if got := o.takeStats().MaxBacklog; got != 0 {
	230	+	t.Fatalf("recovered backlog %d", got)
	231	+	}
	232	+	}


@@ -210,3 +210,23 @@ func TestOutboxQueueWaitIsPerEntry(t *testing.T) {
210	}	210		}
211	})	211		})
212	}	212		}
		213	+
		214	+	// A stalled sender must remain visible in every reporting period, including
		215	+	// when it has taken the queue into its private delivery batch.
		216	+	func TestOutboxStatsSampleOutstandingBacklog(t *testing.T) {
		217	+	o := newOutbox()
		218	+	out := make(chan ports.ClientCommand)
		219	+	for range outboxLimit {
		220	+	o.push(out, ports.FocusWindow{ID: 1}, ports.SecurityState{}, false)
		221	+	}
		222	+	batch := o.take()
		223	+	for range 2 {
		224	+	if got := o.takeStats().MaxBacklog; got != outboxLimit {
		225	+	t.Fatalf("backlog %d, want %d", got, outboxLimit)
		226	+	}
		227	+	}
		228	+	o.recycle(batch)
		229	+	if got := o.takeStats().MaxBacklog; got != 0 {
		230	+	t.Fatalf("recovered backlog %d", got)
		231	+	}
		232	+	}

M internal/core/screens_test.go +0 -3


@@ -647,9 +647,6 @@ func TestFirstTerminalFollowsSetOutputs(t *testing.T) {
647	647		case <-time.After(time.Second):
648	648		t.Fatal("no output command or spawn")
649	649		}
650		-	if v, ok := receive(t, commands).(ports.SlotsPending); !ok \|\| !v.Pending {
651		-	t.Fatalf("expected pending before spawn, got %v", v)
652		-	}
653	650		if req := receive(t, spawn); len(req.Argv) == 0 {
654	651		t.Fatal(req)
655	652		}


@@ -647,9 +647,6 @@ func TestFirstTerminalFollowsSetOutputs(t *testing.T) {
647		case <-time.After(time.Second):	647	case <-time.After(time.Second):
648		t.Fatal("no output command or spawn")	648	t.Fatal("no output command or spawn")
649		}	649	}
650	-	if v, ok := receive(t, commands).(ports.SlotsPending); !ok \|\| !v.Pending {
651	-	t.Fatalf("expected pending before spawn, got %v", v)
652	-	}
653		if req := receive(t, spawn); len(req.Argv) == 0 {	650	if req := receive(t, spawn); len(req.Argv) == 0 {
654		t.Fatal(req)	651	t.Fatal(req)
655		}	652	}

M internal/core/security_review_test.go +2 -7


@@ -77,7 +77,7 @@ func TestExactEpochAdmissionPreservesProducerModifierState(t *testing.T) {
77	77		}
78	78		}
79	79
80		-	func TestSlotsPendingBackpressureSuppressesSpawnAndDefersSlots(t *testing.T) {
	80	+	func TestEpochChangeSuppressesSpawnAndDefersSlots(t *testing.T) {
81	81		c, _, _ := hiddenCaptureCommands(t)
82	82		key := slotKey{workspace: "dev", index: 1}
83	83		c.slots[key] = &slotState{argv: []string{"terminal"}}

@@ -100,10 +100,6 @@ func TestSlotsPendingBackpressureSuppressesSpawnAndDefersSlots(t *testing.T) {
100	100		spawn := make(chan ports.SpawnRequest, 2)
101	101		c.ch.Spawn = spawn
102	102		err := c.spawnSlots(context.Background(), false)
103		-	cmd := reviewReceive(t, commands).(ports.SecurityCommand)
104		-	if _, ok := cmd.Command.(ports.SlotsPending); !ok {
105		-	t.Fatal(cmd)
106		-	}
107	103		if err != nil {
108	104		t.Fatal(err)
109	105		}

@@ -122,7 +118,7 @@ func TestSlotsPendingBackpressureSuppressesSpawnAndDefersSlots(t *testing.T) {
122	118		}
123	119		}
124	120
125		-	func TestTerminalPendingBackpressureSuppressesSpawnAndRestoresRetry(t *testing.T) {
	121	+	func TestTerminalEpochChangeSuppressesSpawnAndRestoresRetry(t *testing.T) {
126	122		c, _, _ := hiddenCaptureCommands(t)
127	123		c.cur().mon.RemoveWindow(1)
128	124		c.cur().mon.RemoveWindow(2)

@@ -147,7 +143,6 @@ func TestTerminalPendingBackpressureSuppressesSpawnAndRestoresRetry(t *testing.T
147	143		spawn := make(chan ports.SpawnRequest, 1)
148	144		c.ch.Spawn = spawn
149	145		err := c.spawnEmpty(context.Background())
150		-	reviewReceive(t, commands)
151	146		if err != nil {
152	147		t.Fatal(err)
153	148		}

 	}
 }
-func TestSlotsPendingBackpressureSuppressesSpawnAndDefersSlots(t *testing.T) {
+func TestEpochChangeSuppressesSpawnAndDefersSlots(t *testing.T) {
 	c, _, _ := hiddenCaptureCommands(t)
 	key := slotKey{workspace: "dev", index: 1}
 	c.slots[key] = &slotState{argv: []string{"terminal"}}
 	spawn := make(chan ports.SpawnRequest, 2)
 	c.ch.Spawn = spawn
 	err := c.spawnSlots(context.Background(), false)
-	cmd := reviewReceive(t, commands).(ports.SecurityCommand)
-	if _, ok := cmd.Command.(ports.SlotsPending); !ok {
-		t.Fatal(cmd)
-	}
 	if err != nil {
 		t.Fatal(err)
 	}
 	}
 }
-func TestTerminalPendingBackpressureSuppressesSpawnAndRestoresRetry(t *testing.T) {
+func TestTerminalEpochChangeSuppressesSpawnAndRestoresRetry(t *testing.T) {
 	c, _, _ := hiddenCaptureCommands(t)
 	c.cur().mon.RemoveWindow(1)
 	c.cur().mon.RemoveWindow(2)
 	spawn := make(chan ports.SpawnRequest, 1)
 	c.ch.Spawn = spawn
 	err := c.spawnEmpty(context.Background())
-	reviewReceive(t, commands)
 	if err != nil {
 		t.Fatal(err)
 	}

M internal/core/spawn_placement_test.go +1 -10

@@ -1,7 +1,6 @@
1	1		package core
2	2
3	3		import (
4		-	"context"
5	4		"strings"
6	5		"testing"
7	6		"time"

@@ -32,15 +31,7 @@ func TestSlotClaimWithoutWorkspace(t *testing.T) {
32	31		if s, _ := c.screenOf(21); s == nil {
33	32		t.Fatal("window did not map normally")
34	33		}
35		-	// Claim status is also reflected on the client command port.
36		-	commands := make(chan ports.ClientCommand, 1)
37		-	c.ch.Commands = commands
38		-	if err := c.publishPending(context.Background()); err != nil {
39		-	t.Fatal(err)
40		-	}
41		-	if got := (<-commands).(ports.SlotsPending); !got.Pending {
42		-	t.Fatal("pending claim was not retained")
43		-	}
	34	+
44	35		}
45	36
46	37		func TestExpiredTerminalTokenMapsNormally(t *testing.T) {

@@ -1,7 +1,6 @@
1		package core	1		package core
2			2
3		import (	3		import (
4	-	"context"
5		"strings"	4		"strings"
6		"testing"	5		"testing"
7		"time"	6		"time"

@@ -32,15 +31,7 @@ func TestSlotClaimWithoutWorkspace(t *testing.T) {
32		if s, _ := c.screenOf(21); s == nil {	31		if s, _ := c.screenOf(21); s == nil {
33		t.Fatal("window did not map normally")	32		t.Fatal("window did not map normally")
34		}	33		}
35	-	// Claim status is also reflected on the client command port.	34	+
36	-	commands := make(chan ports.ClientCommand, 1)
37	-	c.ch.Commands = commands
38	-	if err := c.publishPending(context.Background()); err != nil {
39	-	t.Fatal(err)
40	-	}
41	-	if got := (<-commands).(ports.SlotsPending); !got.Pending {
42	-	t.Fatal("pending claim was not retained")
43	-	}
44		}	35		}
45			36
46		func TestExpiredTerminalTokenMapsNormally(t *testing.T) {	37		func TestExpiredTerminalTokenMapsNormally(t *testing.T) {

M internal/core/stash_spawn_internal_test.go +17 -58


@@ -5,7 +5,6 @@ import (
5	5		"errors"
6	6		"strings"
7	7		"testing"
8		-	"testing/synctest"
9	8		"time"
10	9
11	10		portsmocks "github.com/bnema/neferwl/internal/mocks/ports"

@@ -40,46 +39,23 @@ func claimToken(req ports.SpawnRequest) string {
40	39		return strings.TrimPrefix(req.Env[0], ports.SlotEnv+"=")
41	40		}
42	41
43		-	// Wayland reads tokens only while slots are pending, so the flag is in the
44		-	// command channel before the spawn can reach the launcher, also when the
45		-	// display reads nothing: in a synctest bubble the sender is waited on until
46		-	// it blocks on the unread channel, and the spawn channel must still be empty.
47		-	func TestSpawnStashPublishesPendingBeforeSpawn(t *testing.T) {
48		-	synctest.Test(t, func(t *testing.T) {
49		-	now := claimBase
50		-	commands := make(chan ports.ClientCommand)
51		-	spawn := make(chan ports.SpawnRequest, 1)
52		-	c := stashSpawnCore(t, commands, spawn, fixedClock(t, &now))
53		-	ctx, cancel := context.WithCancel(context.Background())
54		-	t.Cleanup(cancel)
55		-	c.out.start()
56		-	go c.out.run(ctx, commands)
57		-	done := make(chan error, 1)
58		-	go func() { done <- c.spawnStash(ctx, c.cur().mon.Current(), [][]string{{"firefox"}}, false) }()
59		-	synctest.Wait()
60		-	if len(spawn) != 0 {
61		-	t.Fatal("spawned before the pending flag was delivered")
62		-	}
63		-	select {
64		-	case err := <-done:
65		-	t.Fatalf("spawnStash returned before the flag was read: %v", err)
66		-	default:
67		-	}
68		-	if p, ok := (<-commands).(ports.SlotsPending); !ok \|\| !p.Pending {
69		-	t.Fatal("first command is not SlotsPending{true}")
70		-	}
71		-	synctest.Wait()
72		-	if err := <-done; err != nil {
73		-	t.Fatal(err)
74		-	}
75		-	if len(spawn) != 1 {
76		-	t.Fatal("no spawn after the flag")
77		-	}
78		-	})
	42	+	// A stalled command transport must not delay a placement spawn.
	43	+	func TestSpawnStashIndependentOfCommandDrain(t *testing.T) {
	44	+	now := claimBase
	45	+	commands := make(chan ports.ClientCommand)
	46	+	spawn := make(chan ports.SpawnRequest, 1)
	47	+	c := stashSpawnCore(t, commands, spawn, fixedClock(t, &now))
	48	+	if err := c.spawnStash(context.Background(), c.cur().mon.Current(), [][]string{{"firefox"}}, false); err != nil {
	49	+	t.Fatal(err)
	50	+	}
	51	+	req := <-spawn
	52	+	c.placement.place(c, ports.WindowMapped{ID: 5, Slot: claimToken(req)})
	53	+	if w := c.cur().mon.Current(); len(w.Stash) != 1 \|\| w.Stash[0].ID != 5 {
	54	+	t.Fatal("placement lost while commands unread")
	55	+	}
79	56		}
80	57
81		-	// A send the launcher queue suppresses leaves no claim behind, and the
82		-	// pending flag falls again at the next publish.
	58	+	// A send the launcher queue suppresses leaves no claim behind.
83	59		func TestSpawnStashSuppressedLeavesNoClaim(t *testing.T) {
84	60		now := claimBase
85	61		commands := make(chan ports.ClientCommand, 8)

@@ -90,15 +66,6 @@ func TestSpawnStashSuppressedLeavesNoClaim(t *testing.T) {
90	66		if c.placement.anyPending() {
91	67		t.Fatal("suppressed claims stayed pending")
92	68		}
93		-	if v := (<-commands).(ports.SlotsPending); !v.Pending {
94		-	t.Fatal("pending not announced")
95		-	}
96		-	if err := c.publishPending(context.Background()); err != nil {
97		-	t.Fatal(err)
98		-	}
99		-	if v := (<-commands).(ports.SlotsPending); v.Pending {
100		-	t.Fatal("stale pending flag not republished")
101		-	}
102	69		}
103	70
104	71		// From the first command that is not issued, the rest are not sent and their

@@ -112,25 +79,17 @@ func TestSpawnStashCancelsClaimsFromFirstUnissued(t *testing.T) {
112	79		wantErr error
113	80		}{
114	81		{"queue of one", 1, false, 1, nil},
115		-	{"publication fails", 4, true, 0, context.Canceled},
	82	+	{"context cancelled", 4, true, 0, context.Canceled},
116	83		} {
117	84		t.Run(tc.name, func(t *testing.T) {
118	85		now := claimBase
119		-	commands := make(chan ports.ClientCommand) // unread: a send blocks until the context ends
	86	+	commands := make(chan ports.ClientCommand) // unread: spawns are independent of command delivery
120	87		spawn := make(chan ports.SpawnRequest, tc.queue)
121	88		c := stashSpawnCore(t, commands, spawn, fixedClock(t, &now))
122	89		ctx, cancel := context.WithCancel(context.Background())
123	90		t.Cleanup(cancel)
124	91		if tc.cancelled {
125	92		cancel()
126		-	} else {
127		-	// An unread unbuffered channel would block the flag: read it.
128		-	go func() {
129		-	select {
130		-	case <-commands:
131		-	case <-ctx.Done():
132		-	}
133		-	}()
134	93		}
135	94		err := c.spawnStash(ctx, c.cur().mon.Current(), [][]string{{"a"}, {"b"}}, false)
136	95		if !errors.Is(err, tc.wantErr) {

 	"errors"
 	"strings"
 	"testing"
-	"testing/synctest"
 	"time"
 	portsmocks "github.com/bnema/neferwl/internal/mocks/ports"
 	return strings.TrimPrefix(req.Env[0], ports.SlotEnv+"=")
 }
-// Wayland reads tokens only while slots are pending, so the flag is in the
+// A stalled command transport must not delay a placement spawn.
-// command channel before the spawn can reach the launcher, also when the
+func TestSpawnStashIndependentOfCommandDrain(t *testing.T) {
-// display reads nothing: in a synctest bubble the sender is waited on until
+	now := claimBase
-// it blocks on the unread channel, and the spawn channel must still be empty.
+	commands := make(chan ports.ClientCommand)
-func TestSpawnStashPublishesPendingBeforeSpawn(t *testing.T) {
+	spawn := make(chan ports.SpawnRequest, 1)
-	synctest.Test(t, func(t *testing.T) {
+	c := stashSpawnCore(t, commands, spawn, fixedClock(t, &now))
-		now := claimBase
+	if err := c.spawnStash(context.Background(), c.cur().mon.Current(), [][]string{{"firefox"}}, false); err != nil {
-		commands := make(chan ports.ClientCommand)
+		t.Fatal(err)
-		spawn := make(chan ports.SpawnRequest, 1)
+	}
-		c := stashSpawnCore(t, commands, spawn, fixedClock(t, &now))
+	req := <-spawn
-		ctx, cancel := context.WithCancel(context.Background())
+	c.placement.place(c, ports.WindowMapped{ID: 5, Slot: claimToken(req)})
-		t.Cleanup(cancel)
+	if w := c.cur().mon.Current(); len(w.Stash) != 1 || w.Stash[0].ID != 5 {
-		c.out.start()
+		t.Fatal("placement lost while commands unread")
-		go c.out.run(ctx, commands)
+	}
-		done := make(chan error, 1)
-		go func() { done <- c.spawnStash(ctx, c.cur().mon.Current(), [][]string{{"firefox"}}, false) }()
-		synctest.Wait()
-		if len(spawn) != 0 {
-			t.Fatal("spawned before the pending flag was delivered")
-		}
-		select {
-		case err := <-done:
-			t.Fatalf("spawnStash returned before the flag was read: %v", err)
-		default:
-		}
-		if p, ok := (<-commands).(ports.SlotsPending); !ok || !p.Pending {
-			t.Fatal("first command is not SlotsPending{true}")
-		}
-		synctest.Wait()
-		if err := <-done; err != nil {
-			t.Fatal(err)
-		}
-		if len(spawn) != 1 {
-			t.Fatal("no spawn after the flag")
-		}
-	})
 }
-// A send the launcher queue suppresses leaves no claim behind, and the
+// A send the launcher queue suppresses leaves no claim behind.
-// pending flag falls again at the next publish.
 func TestSpawnStashSuppressedLeavesNoClaim(t *testing.T) {
 	now := claimBase
 	commands := make(chan ports.ClientCommand, 8)
 	if c.placement.anyPending() {
 		t.Fatal("suppressed claims stayed pending")
 	}
-	if v := (<-commands).(ports.SlotsPending); !v.Pending {
-		t.Fatal("pending not announced")
-	}
-	if err := c.publishPending(context.Background()); err != nil {
-		t.Fatal(err)
-	}
-	if v := (<-commands).(ports.SlotsPending); v.Pending {
-		t.Fatal("stale pending flag not republished")
-	}
 }
 // From the first command that is not issued, the rest are not sent and their
 		wantErr   error
 	}{
 		{"queue of one", 1, false, 1, nil},
-		{"publication fails", 4, true, 0, context.Canceled},
+		{"context cancelled", 4, true, 0, context.Canceled},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			now := claimBase
-			commands := make(chan ports.ClientCommand) // unread: a send blocks until the context ends
+			commands := make(chan ports.ClientCommand) // unread: spawns are independent of command delivery
 			spawn := make(chan ports.SpawnRequest, tc.queue)
 			c := stashSpawnCore(t, commands, spawn, fixedClock(t, &now))
 			ctx, cancel := context.WithCancel(context.Background())
 			t.Cleanup(cancel)
 			if tc.cancelled {
 				cancel()
-			} else {
-				// An unread unbuffered channel would block the flag: read it.
-				go func() {
-					select {
-					case <-commands:
-					case <-ctx.Done():
-					}
-				}()
 			}
 			err := c.spawnStash(ctx, c.cur().mon.Current(), [][]string{{"a"}, {"b"}}, false)
 			if !errors.Is(err, tc.wantErr) {

M internal/core/stash_spawn_test.go +0 -22


@@ -29,16 +29,6 @@ func slotToken(t *testing.T, req ports.SpawnRequest) string {
29	29		return req.Env[0][len(ports.SlotEnv)+1:]
30	30		}
31	31
32		-	// pendingSent drains the command queue and reports the last SlotsPending.
33		-	func pendingSent(r *multiRig) (pending, sent bool) {
34		-	for len(r.commands) > 0 {
35		-	if v, ok := (<-r.commands).(ports.SlotsPending); ok {
36		-	pending, sent = v.Pending, true
37		-	}
38		-	}
39		-	return pending, sent
40		-	}
41		-
42	32		func sceneWindow(set []ports.Scene, id ports.WindowID) (ports.SceneWindow, bool) {
43	33		for _, s := range set {
44	34		for _, w := range s.Windows {

@@ -62,9 +52,6 @@ func TestStashBindOpensWindowInStash(t *testing.T) {
62	52		t.Fatalf("argv %v", req.Argv)
63	53		}
64	54		token := slotToken(t, req)
65		-	if pending, sent := pendingSent(r); !sent \|\| !pending {
66		-	t.Fatal("SlotsPending{true} was not sent")
67		-	}
68	55		r.key(t, "Next", ports.ModAlt) // focus moves to workspace 2
69	56		r.client <- ports.WindowMapped{ID: 2, Slot: token}
70	57		set := receive(t, r.scenes)

@@ -79,9 +66,6 @@ func TestStashBindOpensWindowInStash(t *testing.T) {
79	66		if w1, _ := sceneWindow(set, 1); w1.Focused {
80	67		t.Fatal("tile kept the focus")
81	68		}
82		-	if pending, sent := pendingSent(r); !sent \|\| pending {
83		-	t.Fatalf("pending flag not cleared: %v %v", pending, sent)
84		-	}
85	69		// Unstashing tiles it as a column of its own.
86	70		set = r.key(t, "s", ports.ModAlt\|ports.ModShift)
87	71		if w, ok := sceneWindow(set, 2); !ok \|\| w.Floating \|\| w.Hidden \|\| !w.Focused {

@@ -112,9 +96,6 @@ func TestToggleEmptyStashOpensTerminal(t *testing.T) {
112	96		t.Fatalf("argv %v", req.Argv)
113	97		}
114	98		token := slotToken(t, req)
115		-	if pending, sent := pendingSent(r); !sent \|\| !pending {
116		-	t.Fatal("SlotsPending{true} was not sent")
117		-	}
118	99		r.key(t, "s", ports.ModAlt)
119	100		if len(r.spawn) != 0 {
120	101		t.Fatal("second terminal while the first is awaited")

@@ -182,9 +163,6 @@ func TestFourFingerSwipeEmptyStashOpensTerminal(t *testing.T) {
182	163		t.Fatalf("argv %v", req.Argv)
183	164		}
184	165		token := slotToken(t, req)
185		-	if pending, sent := pendingSent(r.multiRig); !sent \|\| !pending {
186		-	t.Fatal("SlotsPending{true} was not sent")
187		-	}
188	166		r.four(t, 0, 40, 10)
189	167		if len(r.spawn) != 0 {
190	168		t.Fatal("second terminal while the first is awaited")


@@ -29,16 +29,6 @@ func slotToken(t *testing.T, req ports.SpawnRequest) string {
29		return req.Env[0][len(ports.SlotEnv)+1:]	29	return req.Env[0][len(ports.SlotEnv)+1:]
30		}	30	}
31			31
32	-	// pendingSent drains the command queue and reports the last SlotsPending.
33	-	func pendingSent(r *multiRig) (pending, sent bool) {
34	-	for len(r.commands) > 0 {
35	-	if v, ok := (<-r.commands).(ports.SlotsPending); ok {
36	-	pending, sent = v.Pending, true
37	-	}
38	-	}
39	-	return pending, sent
40	-	}
41	-
42		func sceneWindow(set []ports.Scene, id ports.WindowID) (ports.SceneWindow, bool) {	32	func sceneWindow(set []ports.Scene, id ports.WindowID) (ports.SceneWindow, bool) {
43		for _, s := range set {	33	for _, s := range set {
44		for _, w := range s.Windows {	34	for _, w := range s.Windows {

@@ -62,9 +52,6 @@ func TestStashBindOpensWindowInStash(t *testing.T) {
62		t.Fatalf("argv %v", req.Argv)	52	t.Fatalf("argv %v", req.Argv)
63		}	53	}
64		token := slotToken(t, req)	54	token := slotToken(t, req)
65	-	if pending, sent := pendingSent(r); !sent \|\| !pending {
66	-	t.Fatal("SlotsPending{true} was not sent")
67	-	}
68		r.key(t, "Next", ports.ModAlt) // focus moves to workspace 2	55	r.key(t, "Next", ports.ModAlt) // focus moves to workspace 2
69		r.client <- ports.WindowMapped{ID: 2, Slot: token}	56	r.client <- ports.WindowMapped{ID: 2, Slot: token}
70		set := receive(t, r.scenes)	57	set := receive(t, r.scenes)

@@ -79,9 +66,6 @@ func TestStashBindOpensWindowInStash(t *testing.T) {
79		if w1, _ := sceneWindow(set, 1); w1.Focused {	66	if w1, _ := sceneWindow(set, 1); w1.Focused {
80		t.Fatal("tile kept the focus")	67	t.Fatal("tile kept the focus")
81		}	68	}
82	-	if pending, sent := pendingSent(r); !sent \|\| pending {
83	-	t.Fatalf("pending flag not cleared: %v %v", pending, sent)
84	-	}
85		// Unstashing tiles it as a column of its own.	69	// Unstashing tiles it as a column of its own.
86		set = r.key(t, "s", ports.ModAlt\|ports.ModShift)	70	set = r.key(t, "s", ports.ModAlt\|ports.ModShift)
87		if w, ok := sceneWindow(set, 2); !ok \|\| w.Floating \|\| w.Hidden \|\| !w.Focused {	71	if w, ok := sceneWindow(set, 2); !ok \|\| w.Floating \|\| w.Hidden \|\| !w.Focused {

@@ -112,9 +96,6 @@ func TestToggleEmptyStashOpensTerminal(t *testing.T) {
112		t.Fatalf("argv %v", req.Argv)	96	t.Fatalf("argv %v", req.Argv)
113		}	97	}
114		token := slotToken(t, req)	98	token := slotToken(t, req)
115	-	if pending, sent := pendingSent(r); !sent \|\| !pending {
116	-	t.Fatal("SlotsPending{true} was not sent")
117	-	}
118		r.key(t, "s", ports.ModAlt)	99	r.key(t, "s", ports.ModAlt)
119		if len(r.spawn) != 0 {	100	if len(r.spawn) != 0 {
120		t.Fatal("second terminal while the first is awaited")	101	t.Fatal("second terminal while the first is awaited")

@@ -182,9 +163,6 @@ func TestFourFingerSwipeEmptyStashOpensTerminal(t *testing.T) {
182		t.Fatalf("argv %v", req.Argv)	163	t.Fatalf("argv %v", req.Argv)
183		}	164	}
184		token := slotToken(t, req)	165	token := slotToken(t, req)
185	-	if pending, sent := pendingSent(r.multiRig); !sent \|\| !pending {
186	-	t.Fatal("SlotsPending{true} was not sent")
187	-	}
188		r.four(t, 0, 40, 10)	166	r.four(t, 0, 40, 10)
189		if len(r.spawn) != 0 {	167	if len(r.spawn) != 0 {
190		t.Fatal("second terminal while the first is awaited")	168	t.Fatal("second terminal while the first is awaited")

M internal/ports/client_commands.go +0 -6


@@ -120,12 +120,6 @@ type SetOutputs struct {
120	120
121	121		func (SetOutputs) clientCommand() {}
122	122
123		-	// SlotsPending tells wayland whether a slot waits for its window. Only then
124		-	// does it read the SlotEnv of mapping clients.
125		-	type SlotsPending struct{ Pending bool }
126		-
127		-	func (SlotsPending) clientCommand() {}
128		-
129	123		// CloseWindow carries core → wayland close requests.
130	124		type CloseWindow struct{ ID WindowID }
131	125


@@ -120,12 +120,6 @@ type SetOutputs struct {
120			120
121		func (SetOutputs) clientCommand() {}	121	func (SetOutputs) clientCommand() {}
122			122
123	-	// SlotsPending tells wayland whether a slot waits for its window. Only then
124	-	// does it read the SlotEnv of mapping clients.
125	-	type SlotsPending struct{ Pending bool }
126	-
127	-	func (SlotsPending) clientCommand() {}
128	-
129		// CloseWindow carries core → wayland close requests.	123	// CloseWindow carries core → wayland close requests.
130		type CloseWindow struct{ ID WindowID }	124	type CloseWindow struct{ ID WindowID }
131			125

Comments0 open / 0

no comments yet.

Drag and drop

Commands and stats

Hints and focus flash

Performance

Checks

Merge settings

Comments0 open / 0