fix(app): stop printing retryable broker failures over the retry notice

commit: 3a14a3cf909322da8d3bc661c65d6bcbfb93aa87
author: brice · 28d ago · 2026-09-24 03:38 UTC
parent: 0b823c5
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

2 files · +33 -3

M internal/app/broker_client.go +9 -3


@@ -230,10 +230,16 @@ func (p *brokerClientPresentation) pickerContext() ports.UIContext {
230	230		// notice through the picker, so this is the complementary diagnostic: one
231	231		// bounded line under the shared writer lock, never a raw cause chain and never
232	232		// a competing terminal owner.
233		-	func (p *brokerClientPresentation) Failure(err error) {
	233	+	func (p *brokerClientPresentation) Failure(state client.State, err error) {
234	234		if p == nil \|\| err == nil {
235	235		return
236	236		}
	237	+	// A retryable failure is already shown by the repainted retry notice. A
	238	+	// raw line here would scroll the painted screen on every attempt and
	239	+	// leave a trail of stale notices behind it.
	240	+	if state.Connectivity == client.ConnectivityRetryWait {
	241	+	return
	242	+	}
237	243		text := err.Error()
238	244		if len(text) > ports.BrokerMaxErrorBytes {
239	245		text = text[:ports.BrokerMaxErrorBytes]

@@ -292,12 +298,12 @@ func runBrokerClient(ctx context.Context, cfg brokerClientConfig) error {
292	298		SessionEnvironment: sessionEnv,
293	299		LifecycleActions: cfg.LifecycleActions,
294	300		Render: presentation.Render,
295		-	Notify: func(_ client.State, err error) {
	301	+	Notify: func(state client.State, err error) {
296	302		if cfg.OnFailure != nil {
297	303		cfg.OnFailure(err)
298	304		return
299	305		}
300		-	presentation.Failure(err)
	306	+	presentation.Failure(state, err)
301	307		},
302	308		NotifyLifecycle: func(notice client.LifecycleNotice) {
303	309		if cfg.OnLifecycle != nil {

 // notice through the picker, so this is the complementary diagnostic: one
 // bounded line under the shared writer lock, never a raw cause chain and never
 // a competing terminal owner.
-func (p *brokerClientPresentation) Failure(err error) {
+func (p *brokerClientPresentation) Failure(state client.State, err error) {
 	if p == nil || err == nil {
 		return
 	}
+	// A retryable failure is already shown by the repainted retry notice. A
+	// raw line here would scroll the painted screen on every attempt and
+	// leave a trail of stale notices behind it.
+	if state.Connectivity == client.ConnectivityRetryWait {
+		return
+	}
 	text := err.Error()
 	if len(text) > ports.BrokerMaxErrorBytes {
 		text = text[:ports.BrokerMaxErrorBytes]
 		SessionEnvironment:       sessionEnv,
 		LifecycleActions:         cfg.LifecycleActions,
 		Render:                   presentation.Render,
-		Notify: func(_ client.State, err error) {
+		Notify: func(state client.State, err error) {
 			if cfg.OnFailure != nil {
 				cfg.OnFailure(err)
 				return
 			}
-			presentation.Failure(err)
+			presentation.Failure(state, err)
 		},
 		NotifyLifecycle: func(notice client.LifecycleNotice) {
 			if cfg.OnLifecycle != nil {

M internal/app/broker_offline_client_render_test.go +24 -0


@@ -2,6 +2,7 @@ package app
2	2
3	3		import (
4	4		"bytes"
	5	+	"errors"
5	6		"io"
6	7		"strings"
7	8		"sync"

@@ -308,3 +309,26 @@ func TestOfflineClientPickerPaintCommitsPickerTransaction(t *testing.T) {
308	309		require.Equal(t, 0, terminal.beginDepth, "the transaction is balanced")
309	310		require.Equal(t, []bool{true}, terminal.endSuccesses, "the frame committed after write and flush")
310	311		}
	312	+
	313	+	// TestBrokerClientFailureWritesOnlyNonRetryableErrors pins that a retryable
	314	+	// failure never writes a raw line: the retry notice already shows it, and a
	315	+	// line per attempt would scroll the painted screen and duplicate the notice.
	316	+	func TestBrokerClientFailureWritesOnlyNonRetryableErrors(t *testing.T) {
	317	+	tests := []struct {
	318	+	name string
	319	+	connectivity client.Connectivity
	320	+	wantLine bool
	321	+	}{
	322	+	{name: "retry wait stays silent", connectivity: client.ConnectivityRetryWait},
	323	+	{name: "disconnected is reported", connectivity: client.ConnectivityDisconnected, wantLine: true},
	324	+	}
	325	+	for _, tt := range tests {
	326	+	t.Run(tt.name, func(t *testing.T) {
	327	+	terminal := newOfflineRenderTerminal(domain.Geometry{Size: domain.Size{Cols: 80, Rows: 24}})
	328	+	presentation := &brokerClientPresentation{terminal: terminal}
	329	+	presentation.Failure(client.State{Connectivity: tt.connectivity}, errors.New("broker connection lost"))
	330	+	written, _ := terminal.written()
	331	+	require.Equal(t, tt.wantLine, strings.Contains(written, "broker connection lost"))
	332	+	})
	333	+	}
	334	+	}


@@ -2,6 +2,7 @@ package app
2		2
3	import (	3		import (
4	"bytes"	4		"bytes"
		5	+	"errors"
5	"io"	6		"io"
6	"strings"	7		"strings"
7	"sync"	8		"sync"

@@ -308,3 +309,26 @@ func TestOfflineClientPickerPaintCommitsPickerTransaction(t *testing.T) {
308	require.Equal(t, 0, terminal.beginDepth, "the transaction is balanced")	309		require.Equal(t, 0, terminal.beginDepth, "the transaction is balanced")
309	require.Equal(t, []bool{true}, terminal.endSuccesses, "the frame committed after write and flush")	310		require.Equal(t, []bool{true}, terminal.endSuccesses, "the frame committed after write and flush")
310	}	311		}
		312	+
		313	+	// TestBrokerClientFailureWritesOnlyNonRetryableErrors pins that a retryable
		314	+	// failure never writes a raw line: the retry notice already shows it, and a
		315	+	// line per attempt would scroll the painted screen and duplicate the notice.
		316	+	func TestBrokerClientFailureWritesOnlyNonRetryableErrors(t *testing.T) {
		317	+	tests := []struct {
		318	+	name string
		319	+	connectivity client.Connectivity
		320	+	wantLine bool
		321	+	}{
		322	+	{name: "retry wait stays silent", connectivity: client.ConnectivityRetryWait},
		323	+	{name: "disconnected is reported", connectivity: client.ConnectivityDisconnected, wantLine: true},
		324	+	}
		325	+	for _, tt := range tests {
		326	+	t.Run(tt.name, func(t *testing.T) {
		327	+	terminal := newOfflineRenderTerminal(domain.Geometry{Size: domain.Size{Cols: 80, Rows: 24}})
		328	+	presentation := &brokerClientPresentation{terminal: terminal}
		329	+	presentation.Failure(client.State{Connectivity: tt.connectivity}, errors.New("broker connection lost"))
		330	+	written, _ := terminal.written()
		331	+	require.Equal(t, tt.wantLine, strings.Contains(written, "broker connection lost"))
		332	+	})
		333	+	}
		334	+	}