fix(daemon): serialize toast publication with detach

commit: 4165239d78e9538070d8ae078bc59ccd658b1c43
author: brice · 3mo ago · 2026-07-23 21:38 UTC
parent: 6ea049e
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

2 files · +103 -26

M internal/usecase/daemon/notify.go +46 -26


@@ -41,6 +41,9 @@ type noticeCenter struct {
41	41		// beforeGlobalDelivery is a deterministic test seam after targets have been
42	42		// selected and before their toast stacks are touched.
43	43		beforeGlobalDelivery func()
	44	+	// beforeSessionDelivery is a deterministic test seam after a session client
	45	+	// has been selected and before its toast stack is touched.
	46	+	beforeSessionDelivery func()
44	47		// beforeClientNoticeMutation is a deterministic test seam after a client
45	48		// notice's ownership validation and before its toast state is changed.
46	49		beforeClientNoticeMutation func()

@@ -254,11 +257,16 @@ func (d Daemon) notify(sess session, sev domain.NoticeSeverity, code domain.No
254	257		d.notices.routingMu.Unlock()
255	258		return
256	259		}
257		-	// Live session-scoped notices do not use the pending queue, so release
258		-	// routing before painting. Painting can itself report an error, which
259		-	// may recursively route a notice.
	260	+	if d.notices.beforeSessionDelivery != nil {
	261	+	d.notices.beforeSessionDelivery()
	262	+	}
	263	+	// Publish the toast while detach is excluded, then release routing before
	264	+	// repainting. Rendering can report another notice and re-enter routing.
	265	+	published := d.publishToast(ac, n)
260	266		d.notices.routingMu.Unlock()
261		-	d.showToast(ac, n)
	267	+	if published {
	268	+	d.repaintForNotice(ac)
	269	+	}
262	270		return
263	271		}
264	272

@@ -266,10 +274,9 @@ func (d Daemon) notify(sess session, sev domain.NoticeSeverity, code domain.No
266	274		}
267	275
268	276		// deliverGlobal atomically selects all current attachments and either publishes
269		-	// to all of them or queues the notice. routingMu is deliberately held through
270		-	// showToast, but never with d.mu or sess.mu held: detach cannot turn a selected
271		-	// target stale between selection and publication, and firstPaint cannot drain
272		-	// before an unattached route has queued its notice.
	277	+	// to all of them or queues the notice. routingMu is held through toast-state
	278	+	// publication, but never through repaint: detach cannot turn a selected target
	279	+	// stale before publication, while rendering remains free to re-enter routing.
273	280		func (d *Daemon) deliverGlobal(n domain.Notification) {
274	281		d.mu.Lock()
275	282		d.notices.routingMu.Lock()

@@ -278,28 +285,31 @@ func (d *Daemon) deliverGlobal(n domain.Notification) {
278	285		sessions = append(sessions, s)
279	286		}
280	287		d.mu.Unlock()
281		-	defer d.notices.routingMu.Unlock()
282	288
283	289		if d.notices.beforeGlobalDelivery != nil {
284	290		d.notices.beforeGlobalDelivery()
285	291		}
286		-	delivered := false
	292	+	targets := make([]*attachedClient, 0, len(sessions))
287	293		for _, s := range sessions {
288	294		s.mu.Lock()
289	295		ac := s.client
290	296		s.mu.Unlock()
291		-	if ac == nil {
	297	+	if ac == nil \|\| !d.publishToast(ac, n) {
292	298		continue
293	299		}
294		-	d.showToast(ac, n)
295		-	delivered = true
	300	+	targets = append(targets, ac)
296	301		}
297		-	if !delivered {
	302	+	if len(targets) == 0 {
298	303		if d.notices.beforeQueueGlobal != nil {
299	304		d.notices.beforeQueueGlobal()
300	305		}
301	306		d.notices.queueGlobal(n)
302	307		}
	308	+	d.notices.routingMu.Unlock()
	309	+
	310	+	for _, ac := range targets {
	311	+	d.repaintForNotice(ac)
	312	+	}
303	313		}
304	314
305	315		// drainPendingForFirstPaint publishes pending globals and notices scoped to

@@ -308,18 +318,19 @@ func (d *Daemon) deliverGlobal(n domain.Notification) {
308	318		// drain has already happened.
309	319		func (d Daemon) drainPendingForFirstPaint(sess session, ac *attachedClient) {
310	320		d.notices.routingMu.Lock()
311		-	defer d.notices.routingMu.Unlock()
312	321
313	322		sess.mu.Lock()
314	323		current := sess.client == ac
315	324		sess.mu.Unlock()
316	325		if !current {
	326	+	d.notices.routingMu.Unlock()
317	327		return
318	328		}
	329	+	published := false
319	330		var keep []domain.Notification
320	331		for _, n := range d.notices.drainPending() {
321	332		if n.SessionID == "" \|\| n.SessionID == sess.id {
322		-	d.showToast(ac, n)
	333	+	published = d.publishToast(ac, n) \|\| published
323	334		continue
324	335		}
325	336		keep = append(keep, n)

@@ -327,6 +338,10 @@ func (d Daemon) drainPendingForFirstPaint(sess session, ac *attachedClient) {
327	338		for _, n := range keep {
328	339		d.notices.queueGlobal(n)
329	340		}
	341	+	d.notices.routingMu.Unlock()
	342	+	if published {
	343	+	d.repaintForNotice(ac)
	344	+	}
330	345		}
331	346
332	347		// NotifyGlobal raises a daemon-wide notice from outside the daemon package.

@@ -334,27 +349,33 @@ func (d *Daemon) NotifyGlobal(sev domain.NoticeSeverity, code domain.NoticeCode,
334	349		d.notify(nil, sev, code, msg, cause)
335	350		}
336	351
337		-	// showToast publishes a notice into one client's toast stack. An identical
338		-	// code within the same scope coalesces into the existing entry and restarts its
339		-	// TTL; otherwise the notice is prepended and the stack trimmed. The repaint is
340		-	// issued after noticeMu is released.
	352	+	// showToast publishes a notice into one client's toast stack and repaints only
	353	+	// after the state mutation is complete.
341	354		func (d Daemon) showToast(ac attachedClient, n domain.Notification) {
	355	+	if d.publishToast(ac, n) {
	356	+	d.repaintForNotice(ac)
	357	+	}
	358	+	}
	359	+
	360	+	// publishToast durably mutates one client's toast state without repainting.
	361	+	// Routing paths call it while routingMu excludes detach, then release routingMu
	362	+	// before rendering so a render failure can safely route another notice.
	363	+	func (d Daemon) publishToast(ac attachedClient, n domain.Notification) bool {
342	364		if ac == nil {
343		-	return
	365	+	return false
344	366		}
345	367		ac.initOverlays()
346	368		rt := ac.overlays
347	369
348	370		rt.noticeMu.Lock()
	371	+	defer rt.noticeMu.Unlock()
349	372		if i := rt.indexOfToastLocked(n.Code, n.SessionID); i >= 0 {
350	373		rt.noticeToasts[i].n.Count += n.Count
351	374		rt.noticeToasts[i].n.Time = n.Time
352	375		rt.noticeToasts[i].n.Details = n.Details
353	376		rt.noticeToasts[i].timer.stop()
354	377		d.retainToastTimerLocked(ac, &rt.noticeToasts[i])
355		-	rt.noticeMu.Unlock()
356		-	d.repaintForNotice(ac)
357		-	return
	378	+	return true
358	379		}
359	380
360	381		toast := noticeToast{n: n}

@@ -367,8 +388,7 @@ func (d Daemon) showToast(ac attachedClient, n domain.Notification) {
367	388		rt.noticeToasts = rt.noticeToasts[:maxVisibleToasts]
368	389		}
369	390		d.retainToastTimerLocked(ac, &rt.noticeToasts[0])
370		-	rt.noticeMu.Unlock()
371		-	d.repaintForNotice(ac)
	391	+	return true
372	392		}
373	393
374	394		// indexOfToastLocked finds a visible toast with the same code and scope.

 	// beforeGlobalDelivery is a deterministic test seam after targets have been
 	// selected and before their toast stacks are touched.
 	beforeGlobalDelivery func()
+	// beforeSessionDelivery is a deterministic test seam after a session client
+	// has been selected and before its toast stack is touched.
+	beforeSessionDelivery func()
 	// beforeClientNoticeMutation is a deterministic test seam after a client
 	// notice's ownership validation and before its toast state is changed.
 	beforeClientNoticeMutation func()
 			d.notices.routingMu.Unlock()
 			return
 		}
-		// Live session-scoped notices do not use the pending queue, so release
+		if d.notices.beforeSessionDelivery != nil {
-		// routing before painting. Painting can itself report an error, which
+			d.notices.beforeSessionDelivery()
-		// may recursively route a notice.
+		}
+		// Publish the toast while detach is excluded, then release routing before
+		// repainting. Rendering can report another notice and re-enter routing.
+		published := d.publishToast(ac, n)
 		d.notices.routingMu.Unlock()
-		d.showToast(ac, n)
+		if published {
+			d.repaintForNotice(ac)
+		}
 		return
 	}
 }
 // deliverGlobal atomically selects all current attachments and either publishes
-// to all of them or queues the notice. routingMu is deliberately held through
+// to all of them or queues the notice. routingMu is held through toast-state
-// showToast, but never with d.mu or sess.mu held: detach cannot turn a selected
+// publication, but never through repaint: detach cannot turn a selected target
-// target stale between selection and publication, and firstPaint cannot drain
+// stale before publication, while rendering remains free to re-enter routing.
-// before an unattached route has queued its notice.
 func (d *Daemon) deliverGlobal(n domain.Notification) {
 	d.mu.Lock()
 	d.notices.routingMu.Lock()
 		sessions = append(sessions, s)
 	}
 	d.mu.Unlock()
-	defer d.notices.routingMu.Unlock()
 	if d.notices.beforeGlobalDelivery != nil {
 		d.notices.beforeGlobalDelivery()
 	}
-	delivered := false
+	targets := make([]*attachedClient, 0, len(sessions))
 	for _, s := range sessions {
 		s.mu.Lock()
 		ac := s.client
 		s.mu.Unlock()
-		if ac == nil {
+		if ac == nil || !d.publishToast(ac, n) {
 			continue
 		}
-		d.showToast(ac, n)
+		targets = append(targets, ac)
-		delivered = true
 	}
-	if !delivered {
+	if len(targets) == 0 {
 		if d.notices.beforeQueueGlobal != nil {
 			d.notices.beforeQueueGlobal()
 		}
 		d.notices.queueGlobal(n)
 	}
+	d.notices.routingMu.Unlock()
+	for _, ac := range targets {
+		d.repaintForNotice(ac)
+	}
 }
 // drainPendingForFirstPaint publishes pending globals and notices scoped to
 // drain has already happened.
 func (d *Daemon) drainPendingForFirstPaint(sess *session, ac *attachedClient) {
 	d.notices.routingMu.Lock()
-	defer d.notices.routingMu.Unlock()
 	sess.mu.Lock()
 	current := sess.client == ac
 	sess.mu.Unlock()
 	if !current {
+		d.notices.routingMu.Unlock()
 		return
 	}
+	published := false
 	var keep []domain.Notification
 	for _, n := range d.notices.drainPending() {
 		if n.SessionID == "" || n.SessionID == sess.id {
-			d.showToast(ac, n)
+			published = d.publishToast(ac, n) || published
 			continue
 		}
 		keep = append(keep, n)
 	for _, n := range keep {
 		d.notices.queueGlobal(n)
 	}
+	d.notices.routingMu.Unlock()
+	if published {
+		d.repaintForNotice(ac)
+	}
 }
 // NotifyGlobal raises a daemon-wide notice from outside the daemon package.
 	d.notify(nil, sev, code, msg, cause)
 }
-// showToast publishes a notice into one client's toast stack. An identical
+// showToast publishes a notice into one client's toast stack and repaints only
-// code within the same scope coalesces into the existing entry and restarts its
+// after the state mutation is complete.
-// TTL; otherwise the notice is prepended and the stack trimmed. The repaint is
-// issued after noticeMu is released.
 func (d *Daemon) showToast(ac *attachedClient, n domain.Notification) {
+	if d.publishToast(ac, n) {
+		d.repaintForNotice(ac)
+	}
+}
+// publishToast durably mutates one client's toast state without repainting.
+// Routing paths call it while routingMu excludes detach, then release routingMu
+// before rendering so a render failure can safely route another notice.
+func (d *Daemon) publishToast(ac *attachedClient, n domain.Notification) bool {
 	if ac == nil {
-		return
+		return false
 	}
 	ac.initOverlays()
 	rt := ac.overlays
 	rt.noticeMu.Lock()
+	defer rt.noticeMu.Unlock()
 	if i := rt.indexOfToastLocked(n.Code, n.SessionID); i >= 0 {
 		rt.noticeToasts[i].n.Count += n.Count
 		rt.noticeToasts[i].n.Time = n.Time
 		rt.noticeToasts[i].n.Details = n.Details
 		rt.noticeToasts[i].timer.stop()
 		d.retainToastTimerLocked(ac, &rt.noticeToasts[i])
-		rt.noticeMu.Unlock()
+		return true
-		d.repaintForNotice(ac)
-		return
 	}
 	toast := noticeToast{n: n}
 		rt.noticeToasts = rt.noticeToasts[:maxVisibleToasts]
 	}
 	d.retainToastTimerLocked(ac, &rt.noticeToasts[0])
-	rt.noticeMu.Unlock()
+	return true
-	d.repaintForNotice(ac)
 }
 // indexOfToastLocked finds a visible toast with the same code and scope.

M internal/usecase/daemon/notify_test.go +57 -0


@@ -525,6 +525,63 @@ func TestNotifyRoutesToSessionClientOnly(t *testing.T) {
525	525		require.Empty(t, otherToasts, "another session's client must be untouched by session-scoped notices")
526	526		}
527	527
	528	+	func TestNotifySessionScopedSerializesDetachThroughToastPublication(t *testing.T) {
	529	+	d, sess, ac, _ := newNoticeFixture(t, newNoticeClock())
	530	+
	531	+	selected := make(chan struct{})
	532	+	releasePublication := make(chan struct{})
	533	+	d.notices.beforeSessionDelivery = func() {
	534	+	close(selected)
	535	+	<-releasePublication
	536	+	}
	537	+
	538	+	notifyDone := make(chan struct{})
	539	+	go func() {
	540	+	d.notify(sess, domain.NoticeInfo, domain.NoticeUser, "hello", nil)
	541	+	close(notifyDone)
	542	+	}()
	543	+	<-selected
	544	+
	545	+	detachStarted := make(chan struct{})
	546	+	detachDone := make(chan bool, 1)
	547	+	go func() {
	548	+	close(detachStarted)
	549	+	detachDone <- d.detachIfCurrent(sess, ac)
	550	+	}()
	551	+	<-detachStarted
	552	+	select {
	553	+	case <-detachDone:
	554	+	t.Fatal("detach completed before the selected toast was durably published")
	555	+	default:
	556	+	}
	557	+
	558	+	close(releasePublication)
	559	+	<-notifyDone
	560	+	require.True(t, <-detachDone)
	561	+
	562	+	toasts, _ := visibleToasts(ac)
	563	+	require.Len(t, toasts, 1)
	564	+	require.Equal(t, "hello", toasts[0].Message)
	565	+	require.Empty(t, d.notices.drainPending(), "a toast published before detach must not also be queued")
	566	+	}
	567	+
	568	+	func TestNotifySessionScopedReleasesRoutingBeforeRepaint(t *testing.T) {
	569	+	d, sess, ac, _ := newNoticeFixture(t, newNoticeClock())
	570	+
	571	+	repaintObserved := make(chan bool, 1)
	572	+	ac.renderStages.capture = func() {
	573	+	unlocked := d.notices.routingMu.TryLock()
	574	+	if unlocked {
	575	+	d.notices.routingMu.Unlock()
	576	+	}
	577	+	repaintObserved <- unlocked
	578	+	}
	579	+
	580	+	d.notify(sess, domain.NoticeInfo, domain.NoticeUser, "hello", nil)
	581	+
	582	+	require.True(t, <-repaintObserved, "notice repaint must run without routingMu held")
	583	+	}
	584	+
528	585		func TestSessionScopedNoticeQueuedWhileDetached(t *testing.T) {
529	586		d, sessA, oldA, _ := newNoticeFixture(t, newNoticeClock())
530	587		sessA.mu.Lock()


@@ -525,6 +525,63 @@ func TestNotifyRoutesToSessionClientOnly(t *testing.T) {
525	require.Empty(t, otherToasts, "another session's client must be untouched by session-scoped notices")	525		require.Empty(t, otherToasts, "another session's client must be untouched by session-scoped notices")
526	}	526		}
527		527
		528	+	func TestNotifySessionScopedSerializesDetachThroughToastPublication(t *testing.T) {
		529	+	d, sess, ac, _ := newNoticeFixture(t, newNoticeClock())
		530	+
		531	+	selected := make(chan struct{})
		532	+	releasePublication := make(chan struct{})
		533	+	d.notices.beforeSessionDelivery = func() {
		534	+	close(selected)
		535	+	<-releasePublication
		536	+	}
		537	+
		538	+	notifyDone := make(chan struct{})
		539	+	go func() {
		540	+	d.notify(sess, domain.NoticeInfo, domain.NoticeUser, "hello", nil)
		541	+	close(notifyDone)
		542	+	}()
		543	+	<-selected
		544	+
		545	+	detachStarted := make(chan struct{})
		546	+	detachDone := make(chan bool, 1)
		547	+	go func() {
		548	+	close(detachStarted)
		549	+	detachDone <- d.detachIfCurrent(sess, ac)
		550	+	}()
		551	+	<-detachStarted
		552	+	select {
		553	+	case <-detachDone:
		554	+	t.Fatal("detach completed before the selected toast was durably published")
		555	+	default:
		556	+	}
		557	+
		558	+	close(releasePublication)
		559	+	<-notifyDone
		560	+	require.True(t, <-detachDone)
		561	+
		562	+	toasts, _ := visibleToasts(ac)
		563	+	require.Len(t, toasts, 1)
		564	+	require.Equal(t, "hello", toasts[0].Message)
		565	+	require.Empty(t, d.notices.drainPending(), "a toast published before detach must not also be queued")
		566	+	}
		567	+
		568	+	func TestNotifySessionScopedReleasesRoutingBeforeRepaint(t *testing.T) {
		569	+	d, sess, ac, _ := newNoticeFixture(t, newNoticeClock())
		570	+
		571	+	repaintObserved := make(chan bool, 1)
		572	+	ac.renderStages.capture = func() {
		573	+	unlocked := d.notices.routingMu.TryLock()
		574	+	if unlocked {
		575	+	d.notices.routingMu.Unlock()
		576	+	}
		577	+	repaintObserved <- unlocked
		578	+	}
		579	+
		580	+	d.notify(sess, domain.NoticeInfo, domain.NoticeUser, "hello", nil)
		581	+
		582	+	require.True(t, <-repaintObserved, "notice repaint must run without routingMu held")
		583	+	}
		584	+
528	func TestSessionScopedNoticeQueuedWhileDetached(t *testing.T) {	585		func TestSessionScopedNoticeQueuedWhileDetached(t *testing.T) {
529	d, sessA, oldA, _ := newNoticeFixture(t, newNoticeClock())	586		d, sessA, oldA, _ := newNoticeFixture(t, newNoticeClock())
530	sessA.mu.Lock()	587		sessA.mu.Lock()