475bd98 refactor(client): inline trivial navigation accessors and test navigation state · vev

M internal/usecase/client/supervisor.go +2 -2


@@ -590,7 +590,7 @@ func (s *Supervisor) Run(ctx context.Context) (retErr error) {
590	590		retire := func() {
591	591		s.retirePickerKill()
592	592		s.picker.preview.close(s.cfg.Picker)
593		-	s.nav.clearReady()
	593	+	s.nav.readySub = nil
594	594		if sub != nil {
595	595		sub.Close()
596	596		sub = nil

@@ -659,7 +659,7 @@ func (s *Supervisor) Run(ctx context.Context) (retErr error) {
659	659		}
660	660
661	661		service, sub = result.service, result.sub
662		-	s.nav.setReady(sub)
	662	+	s.nav.readySub = sub
663	663		if s.cfg.Picker != nil {
664	664		// Render the first committed publication immediately, before waiting
665	665		// for the next one; the picker never shows a stale empty catalogue


@@ -590,7 +590,7 @@ func (s *Supervisor) Run(ctx context.Context) (retErr error) {
590		retire := func() {	590		retire := func() {
591		s.retirePickerKill()	591		s.retirePickerKill()
592		s.picker.preview.close(s.cfg.Picker)	592		s.picker.preview.close(s.cfg.Picker)
593	-	s.nav.clearReady()	593	+	s.nav.readySub = nil
594		if sub != nil {	594		if sub != nil {
595		sub.Close()	595		sub.Close()
596		sub = nil	596		sub = nil

@@ -659,7 +659,7 @@ func (s *Supervisor) Run(ctx context.Context) (retErr error) {
659		}	659		}
660			660
661		service, sub = result.service, result.sub	661		service, sub = result.service, result.sub
662	-	s.nav.setReady(sub)	662	+	s.nav.readySub = sub
663		if s.cfg.Picker != nil {	663		if s.cfg.Picker != nil {
664		// Render the first committed publication immediately, before waiting	664		// Render the first committed publication immediately, before waiting
665		// for the next one; the picker never shows a stale empty catalogue	665		// for the next one; the picker never shows a stale empty catalogue

M internal/usecase/client/supervisor_navigation.go +6 -25


@@ -12,22 +12,21 @@ import (
12	12		type navigationState struct {
13	13		// readySub is the adopted connection's subscription while the ready phase
14	14		// runs, so the picker overlay over a live attachment keeps folding broker
15		-	// publications. It is only touched from the run goroutine.
	15	+	// publications.
16	16		readySub ports.BrokerSubscription
17	17		// pendingSwap is the request the picker overlay or a daemon navigation
18		-	// committed to another target while an attachment was live. It is only
19		-	// touched from the run goroutine and consumed by settleAttachment.
	18	+	// committed to another target while an attachment was live. Consumed by
	19	+	// settleAttachment.
20	20		pendingSwap *pickerAttachmentTarget
21	21		// pendingInPlace is the choice the live attachment is switching to in
22		-	// place on its own daemon. A refusal turns it into pendingSwap. Only
23		-	// touched from the run goroutine; cleared when the attachment settles.
	22	+	// place on its own daemon. A refusal turns it into pendingSwap. Cleared
	23	+	// when the attachment settles.
24	24		pendingInPlace *pendingInPlace
25	25		// inPlaceSeq numbers in-place choices so a late outcome for an older one
26	26		// is never applied to a newer one.
27	27		inPlaceSeq uint64
28	28		// routes is the client route ledger published to the serving daemon;
29		-	// routesSent is the attachment that received its latest snapshot. Both are
30		-	// only touched from the run goroutine.
	29	+	// routesSent is the attachment that received its latest snapshot.
31	30		routes *routeLedger
32	31		routesSent AttachmentToken
33	32		}

@@ -40,18 +39,6 @@ func (n *navigationState) brokerChanged() <-chan struct{} {
40	39		return n.readySub.Changed()
41	40		}
42	41
43		-	// setReady records the adopted connection's subscription.
44		-	func (n *navigationState) setReady(sub ports.BrokerSubscription) { n.readySub = sub }
45		-
46		-	// clearReady forgets the adopted connection's subscription.
47		-	func (n *navigationState) clearReady() { n.readySub = nil }
48		-
49		-	// setSwap commits the next attachment target.
50		-	func (n navigationState) setSwap(target pickerAttachmentTarget) { n.pendingSwap = target }
51		-
52		-	// hasSwap reports whether a swap target is already committed.
53		-	func (n *navigationState) hasSwap() bool { return n.pendingSwap != nil }
54		-
55	42		// takeSwap returns and clears the committed swap target.
56	43		func (n navigationState) takeSwap() pickerAttachmentTarget {
57	44		swap := n.pendingSwap

@@ -102,12 +89,6 @@ func (n *navigationState) buildRoutes(snapshot ports.BrokerSnapshot, active rout
102	89		return n.routes.build(snapshot, active)
103	90		}
104	91
105		-	// routesSentTo reports whether token already received a route snapshot.
106		-	func (n *navigationState) routesSentTo(token AttachmentToken) bool { return n.routesSent == token }
107		-
108		-	// markRoutesSent records the attachment that received the latest snapshot.
109		-	func (n *navigationState) markRoutesSent(token AttachmentToken) { n.routesSent = token }
110		-
111	92		// resolveRoute resolves one route reference against the ledger.
112	93		func (n *navigationState) resolveRoute(ref protocol.RouteRef) (routeLedgerTarget, bool) {
113	94		if n.routes == nil \|\| ref.IsZero() {

 type navigationState struct {
 	// readySub is the adopted connection's subscription while the ready phase
 	// runs, so the picker overlay over a live attachment keeps folding broker
-	// publications. It is only touched from the run goroutine.
+	// publications.
 	readySub ports.BrokerSubscription
 	// pendingSwap is the request the picker overlay or a daemon navigation
-	// committed to another target while an attachment was live. It is only
+	// committed to another target while an attachment was live. Consumed by
-	// touched from the run goroutine and consumed by settleAttachment.
+	// settleAttachment.
 	pendingSwap *pickerAttachmentTarget
 	// pendingInPlace is the choice the live attachment is switching to in
-	// place on its own daemon. A refusal turns it into pendingSwap. Only
+	// place on its own daemon. A refusal turns it into pendingSwap. Cleared
-	// touched from the run goroutine; cleared when the attachment settles.
+	// when the attachment settles.
 	pendingInPlace *pendingInPlace
 	// inPlaceSeq numbers in-place choices so a late outcome for an older one
 	// is never applied to a newer one.
 	inPlaceSeq uint64
 	// routes is the client route ledger published to the serving daemon;
-	// routesSent is the attachment that received its latest snapshot. Both are
+	// routesSent is the attachment that received its latest snapshot.
-	// only touched from the run goroutine.
 	routes     *routeLedger
 	routesSent AttachmentToken
 }
 	return n.readySub.Changed()
 }
-// setReady records the adopted connection's subscription.
-func (n *navigationState) setReady(sub ports.BrokerSubscription) { n.readySub = sub }
-// clearReady forgets the adopted connection's subscription.
-func (n *navigationState) clearReady() { n.readySub = nil }
-// setSwap commits the next attachment target.
-func (n *navigationState) setSwap(target *pickerAttachmentTarget) { n.pendingSwap = target }
-// hasSwap reports whether a swap target is already committed.
-func (n *navigationState) hasSwap() bool { return n.pendingSwap != nil }
 // takeSwap returns and clears the committed swap target.
 func (n *navigationState) takeSwap() *pickerAttachmentTarget {
 	swap := n.pendingSwap
 	return n.routes.build(snapshot, active)
 }
-// routesSentTo reports whether token already received a route snapshot.
-func (n *navigationState) routesSentTo(token AttachmentToken) bool { return n.routesSent == token }
-// markRoutesSent records the attachment that received the latest snapshot.
-func (n *navigationState) markRoutesSent(token AttachmentToken) { n.routesSent = token }
 // resolveRoute resolves one route reference against the ledger.
 func (n *navigationState) resolveRoute(ref protocol.RouteRef) (routeLedgerTarget, bool) {
 	if n.routes == nil || ref.IsZero() {

A internal/usecase/client/supervisor_navigation_test.go +66 -0

@@ -0,0 +1,66 @@
	1	+	package client
	2	+
	3	+	import (
	4	+	"testing"
	5	+
	6	+	"github.com/bnema/vev/internal/ports"
	7	+	"github.com/bnema/vev/internal/protocol"
	8	+	)
	9	+
	10	+	func TestNavigationState(t *testing.T) {
	11	+	tests := []struct {
	12	+	name string
	13	+	run func(t *testing.T)
	14	+	}{
	15	+	{"takeInPlace stale seq keeps pending", func(t *testing.T) {
	16	+	var n navigationState
	17	+	n.startInPlace(2, pickerAttachmentTarget{})
	18	+	if got := n.takeInPlace(1); got != nil {
	19	+	t.Fatalf("stale takeInPlace = %v, want nil", got)
	20	+	}
	21	+	if n.inPlaceIdle() {
	22	+	t.Fatal("stale takeInPlace cleared pending")
	23	+	}
	24	+	}},
	25	+	{"takeInPlace matching seq clears", func(t *testing.T) {
	26	+	var n navigationState
	27	+	n.startInPlace(2, pickerAttachmentTarget{})
	28	+	if got := n.takeInPlace(2); got == nil \|\| got.seq != 2 {
	29	+	t.Fatalf("takeInPlace = %v, want seq 2", got)
	30	+	}
	31	+	if !n.inPlaceIdle() {
	32	+	t.Fatal("matching takeInPlace kept pending")
	33	+	}
	34	+	}},
	35	+	{"resolveRoute zero value", func(t *testing.T) {
	36	+	var n navigationState
	37	+	if _, ok := n.resolveRoute(protocol.RouteRef{Key: 1, Generation: 1}); ok {
	38	+	t.Fatal("resolveRoute on zero value = true, want false")
	39	+	}
	40	+	}},
	41	+	{"buildRoutes creates ledger lazily", func(t *testing.T) {
	42	+	var n navigationState
	43	+	if n.routes != nil {
	44	+	t.Fatal("routes non-nil at zero value")
	45	+	}
	46	+	n.buildRoutes(ports.BrokerSnapshot{}, routeActive{})
	47	+	if n.routes == nil {
	48	+	t.Fatal("routes nil after buildRoutes")
	49	+	}
	50	+	}},
	51	+	{"takeSwap returns and clears", func(t *testing.T) {
	52	+	var n navigationState
	53	+	want := &pickerAttachmentTarget{}
	54	+	n.pendingSwap = want
	55	+	if got := n.takeSwap(); got != want {
	56	+	t.Fatalf("takeSwap = %p, want %p", got, want)
	57	+	}
	58	+	if n.pendingSwap != nil {
	59	+	t.Fatal("takeSwap kept swap")
	60	+	}
	61	+	}},
	62	+	}
	63	+	for _, tt := range tests {
	64	+	t.Run(tt.name, tt.run)
	65	+	}
	66	+	}

@@ -0,0 +1,66 @@
	1	+	package client
	2	+
	3	+	import (
	4	+	"testing"
	5	+
	6	+	"github.com/bnema/vev/internal/ports"
	7	+	"github.com/bnema/vev/internal/protocol"
	8	+	)
	9	+
	10	+	func TestNavigationState(t *testing.T) {
	11	+	tests := []struct {
	12	+	name string
	13	+	run func(t *testing.T)
	14	+	}{
	15	+	{"takeInPlace stale seq keeps pending", func(t *testing.T) {
	16	+	var n navigationState
	17	+	n.startInPlace(2, pickerAttachmentTarget{})
	18	+	if got := n.takeInPlace(1); got != nil {
	19	+	t.Fatalf("stale takeInPlace = %v, want nil", got)
	20	+	}
	21	+	if n.inPlaceIdle() {
	22	+	t.Fatal("stale takeInPlace cleared pending")
	23	+	}
	24	+	}},
	25	+	{"takeInPlace matching seq clears", func(t *testing.T) {
	26	+	var n navigationState
	27	+	n.startInPlace(2, pickerAttachmentTarget{})
	28	+	if got := n.takeInPlace(2); got == nil \|\| got.seq != 2 {
	29	+	t.Fatalf("takeInPlace = %v, want seq 2", got)
	30	+	}
	31	+	if !n.inPlaceIdle() {
	32	+	t.Fatal("matching takeInPlace kept pending")
	33	+	}
	34	+	}},
	35	+	{"resolveRoute zero value", func(t *testing.T) {
	36	+	var n navigationState
	37	+	if _, ok := n.resolveRoute(protocol.RouteRef{Key: 1, Generation: 1}); ok {
	38	+	t.Fatal("resolveRoute on zero value = true, want false")
	39	+	}
	40	+	}},
	41	+	{"buildRoutes creates ledger lazily", func(t *testing.T) {
	42	+	var n navigationState
	43	+	if n.routes != nil {
	44	+	t.Fatal("routes non-nil at zero value")
	45	+	}
	46	+	n.buildRoutes(ports.BrokerSnapshot{}, routeActive{})
	47	+	if n.routes == nil {
	48	+	t.Fatal("routes nil after buildRoutes")
	49	+	}
	50	+	}},
	51	+	{"takeSwap returns and clears", func(t *testing.T) {
	52	+	var n navigationState
	53	+	want := &pickerAttachmentTarget{}
	54	+	n.pendingSwap = want
	55	+	if got := n.takeSwap(); got != want {
	56	+	t.Fatalf("takeSwap = %p, want %p", got, want)
	57	+	}
	58	+	if n.pendingSwap != nil {
	59	+	t.Fatal("takeSwap kept swap")
	60	+	}
	61	+	}},
	62	+	}
	63	+	for _, tt := range tests {
	64	+	t.Run(tt.name, tt.run)
	65	+	}
	66	+	}

M internal/usecase/client/supervisor_overlay.go +2 -2


@@ -179,7 +179,7 @@ func (o *attachmentPickerOverlay) takeOp() {
179	179		o.release(false)
180	180		return
181	181		}
182		-	s.nav.setSwap(&target)
	182	+	s.nav.pendingSwap = &target
183	183		o.swapping = true
184	184		s.picker.preview.close(o.picker())
185	185		s.attachments.requestDetach(o.run.token)

@@ -281,7 +281,7 @@ func (o *attachmentPickerOverlay) inPlaceSettled(result inPlaceResult) {
281	281		if o.active {
282	282		o.exit()
283	283		}
284		-	s.nav.setSwap(&pending.target)
	284	+	s.nav.pendingSwap = &pending.target
285	285		o.swapping = true
286	286		s.attachments.requestDetach(o.run.token)
287	287		}


@@ -179,7 +179,7 @@ func (o *attachmentPickerOverlay) takeOp() {
179		o.release(false)	179		o.release(false)
180		return	180		return
181		}	181		}
182	-	s.nav.setSwap(&target)	182	+	s.nav.pendingSwap = &target
183		o.swapping = true	183		o.swapping = true
184		s.picker.preview.close(o.picker())	184		s.picker.preview.close(o.picker())
185		s.attachments.requestDetach(o.run.token)	185		s.attachments.requestDetach(o.run.token)

@@ -281,7 +281,7 @@ func (o *attachmentPickerOverlay) inPlaceSettled(result inPlaceResult) {
281		if o.active {	281		if o.active {
282		o.exit()	282		o.exit()
283		}	283		}
284	-	s.nav.setSwap(&pending.target)	284	+	s.nav.pendingSwap = &pending.target
285		o.swapping = true	285		o.swapping = true
286		s.attachments.requestDetach(o.run.token)	286		s.attachments.requestDetach(o.run.token)
287		}	287		}

M internal/usecase/client/supervisor_picker_session.go +3 -2


@@ -3,8 +3,9 @@ package client
3	3		// pickerSession groups the picker state the supervisor carries across its run:
4	4		// the commit observed while connecting, the selected row's preview
5	5		// subscription, and the runner for the picker's kill operations. Like the
6		-	// fields it owns, it is only touched from the run goroutine. The zero value is
7		-	// ready once preview has its clock.
	6	+	// fields it owns, it is only touched from the run goroutine, except the kill
	7	+	// goroutine's kills.wg/kills.done handoff. Construct with preview.clock set;
	8	+	// other fields are ready at zero.
8	9		type pickerSession struct {
9	10		// pendingPickerKey retains a commit observed while connecting. It is
10	11		// revalidated against the adopted service just like a ready-phase commit.

 // pickerSession groups the picker state the supervisor carries across its run:
 // the commit observed while connecting, the selected row's preview
 // subscription, and the runner for the picker's kill operations. Like the
-// fields it owns, it is only touched from the run goroutine. The zero value is
+// fields it owns, it is only touched from the run goroutine, except the kill
-// ready once preview has its clock.
+// goroutine's kills.wg/kills.done handoff. Construct with preview.clock set;
+// other fields are ready at zero.
 type pickerSession struct {
 	// pendingPickerKey retains a commit observed while connecting. It is
 	// revalidated against the adopted service just like a ready-phase commit.

M internal/usecase/client/supervisor_routes.go +7 -11


@@ -39,11 +39,11 @@ func (s Supervisor) publishRoutes(service ports.BrokerNavigator, run attachmen
39	39		return
40	40		}
41	41		snapshot, changed := s.nav.buildRoutes(service.Snapshot(), routeActive{known: true, authority: requestAuthority(request), target: target})
42		-	if snapshot.Generation == 0 \|\| (!changed && s.nav.routesSentTo(run.token)) {
	42	+	if snapshot.Generation == 0 \|\| (!changed && s.nav.routesSent == run.token) {
43	43		return
44	44		}
45	45		if s.attachments.publishRoutes(run.token, snapshot) {
46		-	s.nav.markRoutesSent(run.token)
	46	+	s.nav.routesSent = run.token
47	47		}
48	48		}
49	49

@@ -85,7 +85,7 @@ func (s *Supervisor) settleDaemonNavigation(service ports.BrokerNavigator, overl
85	85		if overlay.active {
86	86		overlay.exit()
87	87		}
88		-	s.nav.setSwap(&target)
	88	+	s.nav.pendingSwap = &target
89	89		overlay.swapping = true
90	90		s.attachments.requestDetach(overlay.run.token)
91	91		}

@@ -108,14 +108,14 @@ func navigationCauseActionID(message protocol.ServerMessage) uint64 {
108	108		// daemon detached the source right after sending it.
109	109		func (s *Supervisor) takeSettledNavigation(service ports.BrokerNavigator, token AttachmentToken, request ports.BrokerOpenStreamRequest) {
110	110		message, ok := s.attachments.takeNavigation()
111		-	if !ok \|\| s.nav.hasSwap() {
	111	+	if !ok \|\| s.nav.pendingSwap != nil {
112	112		return
113	113		}
114	114		if target, ok := s.resolveDaemonNavigation(service, token, request, message); ok {
115	115		if actionID := navigationCauseActionID(message); actionID != 0 && s.cfg.UI != nil {
116	116		s.cfg.UI.follow(s.cfg.UI.generation, actionID)
117	117		}
118		-	s.nav.setSwap(&target)
	118	+	s.nav.pendingSwap = &target
119	119		}
120	120		}
121	121

@@ -128,7 +128,7 @@ func (s *Supervisor) resolveDaemonNavigation(service ports.BrokerNavigator, toke
128	128		s.attachments.replyNavigation(token, protocol.RouteNavigationFailure{Key: typed.Key, Generation: typed.Generation, Code: code})
129	129		return pickerAttachmentTarget{}, false
130	130		}
131		-	routed, ok := s.resolveRoute(protocol.RouteRef{Key: typed.Key, Generation: typed.Generation})
	131	+	routed, ok := s.nav.resolveRoute(protocol.RouteRef{Key: typed.Key, Generation: typed.Generation})
132	132		if !ok \|\| routed.host {
133	133		return fail(protocol.RouteFailureStaleSelection)
134	134		}

@@ -145,7 +145,7 @@ func (s *Supervisor) resolveDaemonNavigation(service ports.BrokerNavigator, toke
145	145		s.attachments.replyNavigation(token, protocol.SessionCreationFailure{RequestID: typed.RequestID, Code: code})
146	146		return pickerAttachmentTarget{}, false
147	147		}
148		-	routed, ok := s.resolveRoute(protocol.RouteRef{Key: typed.Key, Generation: typed.Generation})
	148	+	routed, ok := s.nav.resolveRoute(protocol.RouteRef{Key: typed.Key, Generation: typed.Generation})
149	149		if !ok {
150	150		return fail(protocol.RouteFailureStaleSelection)
151	151		}

@@ -191,10 +191,6 @@ func (s *Supervisor) resolveServingHandoff(service ports.BrokerNavigator, reques
191	191		return pickerAttachmentTarget{request: next, tab: attachmentTab{preferred: handoff.PreferredTabID}}, true
192	192		}
193	193
194		-	func (s *Supervisor) resolveRoute(ref protocol.RouteRef) (routeLedgerTarget, bool) {
195		-	return s.nav.resolveRoute(ref)
196		-	}
197		-
198	194		// resolveRouteSelection resolves one route through the same catalogue rules
199	195		// as a picker commit, against the latest broker publication.
200	196		func (s *Supervisor) resolveRouteSelection(service ports.BrokerNavigator, selection pickerSelectionRef) (pickerAttachmentTarget, error) {

 		return
 	}
 	snapshot, changed := s.nav.buildRoutes(service.Snapshot(), routeActive{known: true, authority: requestAuthority(request), target: target})
-	if snapshot.Generation == 0 || (!changed && s.nav.routesSentTo(run.token)) {
+	if snapshot.Generation == 0 || (!changed && s.nav.routesSent == run.token) {
 		return
 	}
 	if s.attachments.publishRoutes(run.token, snapshot) {
-		s.nav.markRoutesSent(run.token)
+		s.nav.routesSent = run.token
 	}
 }
 	if overlay.active {
 		overlay.exit()
 	}
-	s.nav.setSwap(&target)
+	s.nav.pendingSwap = &target
 	overlay.swapping = true
 	s.attachments.requestDetach(overlay.run.token)
 }
 // daemon detached the source right after sending it.
 func (s *Supervisor) takeSettledNavigation(service ports.BrokerNavigator, token AttachmentToken, request ports.BrokerOpenStreamRequest) {
 	message, ok := s.attachments.takeNavigation()
-	if !ok || s.nav.hasSwap() {
+	if !ok || s.nav.pendingSwap != nil {
 		return
 	}
 	if target, ok := s.resolveDaemonNavigation(service, token, request, message); ok {
 		if actionID := navigationCauseActionID(message); actionID != 0 && s.cfg.UI != nil {
 			s.cfg.UI.follow(s.cfg.UI.generation, actionID)
 		}
-		s.nav.setSwap(&target)
+		s.nav.pendingSwap = &target
 	}
 }
 			s.attachments.replyNavigation(token, protocol.RouteNavigationFailure{Key: typed.Key, Generation: typed.Generation, Code: code})
 			return pickerAttachmentTarget{}, false
 		}
-		routed, ok := s.resolveRoute(protocol.RouteRef{Key: typed.Key, Generation: typed.Generation})
+		routed, ok := s.nav.resolveRoute(protocol.RouteRef{Key: typed.Key, Generation: typed.Generation})
 		if !ok || routed.host {
 			return fail(protocol.RouteFailureStaleSelection)
 		}
 			s.attachments.replyNavigation(token, protocol.SessionCreationFailure{RequestID: typed.RequestID, Code: code})
 			return pickerAttachmentTarget{}, false
 		}
-		routed, ok := s.resolveRoute(protocol.RouteRef{Key: typed.Key, Generation: typed.Generation})
+		routed, ok := s.nav.resolveRoute(protocol.RouteRef{Key: typed.Key, Generation: typed.Generation})
 		if !ok {
 			return fail(protocol.RouteFailureStaleSelection)
 		}
 	return pickerAttachmentTarget{request: next, tab: attachmentTab{preferred: handoff.PreferredTabID}}, true
 }
-func (s *Supervisor) resolveRoute(ref protocol.RouteRef) (routeLedgerTarget, bool) {
-	return s.nav.resolveRoute(ref)
-}
 // resolveRouteSelection resolves one route through the same catalogue rules
 // as a picker commit, against the latest broker publication.
 func (s *Supervisor) resolveRouteSelection(service ports.BrokerNavigator, selection pickerSelectionRef) (pickerAttachmentTarget, error) {