812d54f fix(daemon): close pre-pr concurrency gaps · vev

M internal/usecase/confirm/confirm.go +1 -5


@@ -27,7 +27,7 @@ func (c Confirmer) Confirm(question string) (bool, error) {
27	27		}
28	28		}
29	29		answer, err := bufio.NewReader(c.in).ReadString('\n')
30		-	if err != nil && !isEOFWithAnswer(err, answer) {
	30	+	if err != nil && err != io.EOF {
31	31		return false, err
32	32		}
33	33		switch strings.ToLower(strings.TrimSpace(answer)) {

@@ -37,7 +37,3 @@ func (c Confirmer) Confirm(question string) (bool, error) {
37	37		return false, nil
38	38		}
39	39		}
40		-
41		-	func isEOFWithAnswer(err error, answer string) bool {
42		-	return err == io.EOF && answer != ""
43		-	}

 		}
 	}
 	answer, err := bufio.NewReader(c.in).ReadString('\n')
-	if err != nil && !isEOFWithAnswer(err, answer) {
+	if err != nil && err != io.EOF {
 		return false, err
 	}
 	switch strings.ToLower(strings.TrimSpace(answer)) {
 		return false, nil
 	}
 }
-func isEOFWithAnswer(err error, answer string) bool {
-	return err == io.EOF && answer != ""
-}

M internal/usecase/confirm/confirm_test.go +10 -0


@@ -30,6 +30,16 @@ func TestConfirmerDefaultsNo(t *testing.T) {
30	30		}
31	31		}
32	32
	33	+	func TestConfirmerDefaultsNoOnEOF(t *testing.T) {
	34	+	ok, err := NewConfirmer(strings.NewReader(""), &bytes.Buffer{}).Confirm("Kill daemon?")
	35	+	if err != nil {
	36	+	t.Fatalf("Confirm returned error: %v", err)
	37	+	}
	38	+	if ok {
	39	+	t.Fatal("Confirm returned true for EOF, want false")
	40	+	}
	41	+	}
	42	+
33	43		func TestConfirmerRejectsUnknownAnswer(t *testing.T) {
34	44		ok, err := NewConfirmer(strings.NewReader("later\n"), &bytes.Buffer{}).Confirm("Kill daemon?")
35	45		if err != nil {


@@ -30,6 +30,16 @@ func TestConfirmerDefaultsNo(t *testing.T) {
30	}	30		}
31	}	31		}
32		32
		33	+	func TestConfirmerDefaultsNoOnEOF(t *testing.T) {
		34	+	ok, err := NewConfirmer(strings.NewReader(""), &bytes.Buffer{}).Confirm("Kill daemon?")
		35	+	if err != nil {
		36	+	t.Fatalf("Confirm returned error: %v", err)
		37	+	}
		38	+	if ok {
		39	+	t.Fatal("Confirm returned true for EOF, want false")
		40	+	}
		41	+	}
		42	+
33	func TestConfirmerRejectsUnknownAnswer(t *testing.T) {	43		func TestConfirmerRejectsUnknownAnswer(t *testing.T) {
34	ok, err := NewConfirmer(strings.NewReader("later\n"), &bytes.Buffer{}).Confirm("Kill daemon?")	44		ok, err := NewConfirmer(strings.NewReader("later\n"), &bytes.Buffer{}).Confirm("Kill daemon?")
35	if err != nil {	45		if err != nil {

M internal/usecase/daemon/client.go +11 -8


@@ -222,7 +222,7 @@ func (d *Daemon) waitNotifies() {
222	222		}
223	223		}
224	224
225		-	func (d Daemon) attachClient(sess session, tr ports.Transport, sz domain.Size) *attachedClient {
	225	+	func (d Daemon) attachClient(sess session, tr ports.Transport, sz domain.Size) (attachedClient, attachedClient) {
226	226		ac := &attachedClient{
227	227		tr: tr,
228	228		rend: renderer.New(renderer.Capabilities{}),

@@ -234,15 +234,18 @@ func (d Daemon) attachClient(sess session, tr ports.Transport, sz domain.Size)
234	234		old := sess.client
235	235		sess.client = ac
236	236		sess.mu.Unlock()
	237	+	return ac, old
	238	+	}
237	239
238		-	if old != nil {
239		-	d.unregisterPreview(old)
240		-	old.setSession(nil)
241		-	// Async + bounded: a dead or wedged old client must not stall the new
242		-	// client's handshake.
243		-	d.notifyDetachedAsync(old, ports.ReasonDetach)
	240	+	func (d Daemon) detachReplacedClient(old attachedClient) {
	241	+	if old == nil {
	242	+	return
244	243		}
245		-	return ac
	244	+	d.unregisterPreview(old)
	245	+	old.setSession(nil)
	246	+	// Async + bounded: a dead or wedged old client must not stall the new
	247	+	// client's handshake.
	248	+	d.notifyDetachedAsync(old, ports.ReasonDetach)
246	249		}
247	250
248	251		// firstPaint guarantees the freshly attached client sees the full screen: if

 	}
 }
-func (d *Daemon) attachClient(sess *session, tr ports.Transport, sz domain.Size) *attachedClient {
+func (d *Daemon) attachClient(sess *session, tr ports.Transport, sz domain.Size) (*attachedClient, *attachedClient) {
 	ac := &attachedClient{
 		tr:   tr,
 		rend: renderer.New(renderer.Capabilities{}),
 	old := sess.client
 	sess.client = ac
 	sess.mu.Unlock()
+	return ac, old
+}
-	if old != nil {
+func (d *Daemon) detachReplacedClient(old *attachedClient) {
-		d.unregisterPreview(old)
+	if old == nil {
-		old.setSession(nil)
+		return
-		// Async + bounded: a dead or wedged old client must not stall the new
-		// client's handshake.
-		d.notifyDetachedAsync(old, ports.ReasonDetach)
 	}
-	return ac
+	d.unregisterPreview(old)
+	old.setSession(nil)
+	// Async + bounded: a dead or wedged old client must not stall the new
+	// client's handshake.
+	d.notifyDetachedAsync(old, ports.ReasonDetach)
 }
 // firstPaint guarantees the freshly attached client sees the full screen: if

M internal/usecase/daemon/copymode.go +2 -2


@@ -218,7 +218,7 @@ func (d Daemon) stopCopyPendingTimerLocked(ac attachedClient) {
218	218		}
219	219
220	220		func routeCopyEscape(m *scopy.Mode, snap scopy.Snapshot, data []byte) (int, bool) {
221		-	if len(data) >= 3 && data[1] == '[' {
	221	+	if len(data) >= 3 && (data[1] == '[' \|\| data[1] == 'O') {
222	222		switch data[2] {
223	223		case 'A':
224	224		m.Move(snap, -1)

@@ -242,7 +242,7 @@ func routeCopyEscape(m *scopy.Mode, snap scopy.Snapshot, data []byte) (int, bool
242	242		}
243	243
244	244		func isCopyEscapePrefix(data []byte) bool {
245		-	return len(data) == 2 && data[0] == 0x1b && data[1] == '[' \|\|
	245	+	return len(data) == 2 && data[0] == 0x1b && (data[1] == '[' \|\| data[1] == 'O') \|\|
246	246		len(data) == 3 && data[0] == 0x1b && data[1] == '[' && (data[2] == '5' \|\| data[2] == '6')
247	247		}
248	248


@@ -218,7 +218,7 @@ func (d Daemon) stopCopyPendingTimerLocked(ac attachedClient) {
218		}	218		}
219			219
220		func routeCopyEscape(m *scopy.Mode, snap scopy.Snapshot, data []byte) (int, bool) {	220		func routeCopyEscape(m *scopy.Mode, snap scopy.Snapshot, data []byte) (int, bool) {
221	-	if len(data) >= 3 && data[1] == '[' {	221	+	if len(data) >= 3 && (data[1] == '[' \|\| data[1] == 'O') {
222		switch data[2] {	222		switch data[2] {
223		case 'A':	223		case 'A':
224		m.Move(snap, -1)	224		m.Move(snap, -1)

@@ -242,7 +242,7 @@ func routeCopyEscape(m *scopy.Mode, snap scopy.Snapshot, data []byte) (int, bool
242		}	242		}
243			243
244		func isCopyEscapePrefix(data []byte) bool {	244		func isCopyEscapePrefix(data []byte) bool {
245	-	return len(data) == 2 && data[0] == 0x1b && data[1] == '[' \|\|	245	+	return len(data) == 2 && data[0] == 0x1b && (data[1] == '[' \|\| data[1] == 'O') \|\|
246		len(data) == 3 && data[0] == 0x1b && data[1] == '[' && (data[2] == '5' \|\| data[2] == '6')	246		len(data) == 3 && data[0] == 0x1b && data[1] == '[' && (data[2] == '5' \|\| data[2] == '6')
247		}	247		}
248			248

M internal/usecase/daemon/copymode_test.go +1 -0


@@ -211,6 +211,7 @@ func TestCopyModeSplitArrowDoesNotExit(t *testing.T) {
211	211		{name: "escape then up arrow", input: [][]byte{[]byte("\x1b"), []byte("[A")}, wantCursor: 23},
212	212		{name: "escape then down arrow", input: [][]byte{[]byte("g"), []byte("\x1b"), []byte("[B")}, wantCursor: 1},
213	213		{name: "split up arrow", input: [][]byte{[]byte("\x1b["), []byte("A")}, wantCursor: 23},
	214	+	{name: "split SS3 up arrow", input: [][]byte{[]byte("\x1bO"), []byte("A")}, wantCursor: 23},
214	215		{name: "split page down", input: [][]byte{[]byte("g"), []byte("\x1b[6"), []byte("~")}, wantCursor: 23},
215	216		}
216	217		for _, tc := range cases {


@@ -211,6 +211,7 @@ func TestCopyModeSplitArrowDoesNotExit(t *testing.T) {
211	{name: "escape then up arrow", input: [][]byte{[]byte("\x1b"), []byte("[A")}, wantCursor: 23},	211		{name: "escape then up arrow", input: [][]byte{[]byte("\x1b"), []byte("[A")}, wantCursor: 23},
212	{name: "escape then down arrow", input: [][]byte{[]byte("g"), []byte("\x1b"), []byte("[B")}, wantCursor: 1},	212		{name: "escape then down arrow", input: [][]byte{[]byte("g"), []byte("\x1b"), []byte("[B")}, wantCursor: 1},
213	{name: "split up arrow", input: [][]byte{[]byte("\x1b["), []byte("A")}, wantCursor: 23},	213		{name: "split up arrow", input: [][]byte{[]byte("\x1b["), []byte("A")}, wantCursor: 23},
		214	+	{name: "split SS3 up arrow", input: [][]byte{[]byte("\x1bO"), []byte("A")}, wantCursor: 23},
214	{name: "split page down", input: [][]byte{[]byte("g"), []byte("\x1b[6"), []byte("~")}, wantCursor: 23},	215		{name: "split page down", input: [][]byte{[]byte("g"), []byte("\x1b[6"), []byte("~")}, wantCursor: 23},
215	}	216		}
216	for _, tc := range cases {	217		for _, tc := range cases {

M internal/usecase/daemon/daemon.go +15 -6


@@ -359,11 +359,14 @@ func (d Daemon) route(h ports.Hello, tr ports.Transport) (session, *attachedCl
359	359		case ports.IntentEphemeral:
360	360		name := d.allocEphemeralNameLocked()
361	361		sess, err := d.createSessionLocked(name, true, h.Cwd, sz)
362		-	d.mu.Unlock()
363	362		if err != nil {
	363	+	d.mu.Unlock()
364	364		return nil, nil, err
365	365		}
366		-	return sess, d.attachClient(sess, tr, sz), nil
	366	+	ac, old := d.attachClient(sess, tr, sz)
	367	+	d.mu.Unlock()
	368	+	d.detachReplacedClient(old)
	369	+	return sess, ac, nil
367	370
368	371		case ports.IntentNew:
369	372		if h.Name == "" {

@@ -375,19 +378,25 @@ func (d Daemon) route(h ports.Hello, tr ports.Transport) (session, *attachedCl
375	378		return nil, nil, &protoErr{ports.ErrNameTaken, "session name already in use: " + h.Name}
376	379		}
377	380		sess, err := d.createSessionLocked(h.Name, false, h.Cwd, sz)
378		-	d.mu.Unlock()
379	381		if err != nil {
	382	+	d.mu.Unlock()
380	383		return nil, nil, err
381	384		}
382		-	return sess, d.attachClient(sess, tr, sz), nil
	385	+	ac, old := d.attachClient(sess, tr, sz)
	386	+	d.mu.Unlock()
	387	+	d.detachReplacedClient(old)
	388	+	return sess, ac, nil
383	389
384	390		case ports.IntentAttach:
385	391		sess := d.findByNameLocked(h.Name)
386		-	d.mu.Unlock()
387	392		if sess == nil {
	393	+	d.mu.Unlock()
388	394		return nil, nil, &protoErr{ports.ErrNoSuchSession, "no such session: " + h.Name}
389	395		}
390		-	return sess, d.attachClient(sess, tr, sz), nil
	396	+	ac, old := d.attachClient(sess, tr, sz)
	397	+	d.mu.Unlock()
	398	+	d.detachReplacedClient(old)
	399	+	return sess, ac, nil
391	400
392	401		default:
393	402		d.mu.Unlock()

 	case ports.IntentEphemeral:
 		name := d.allocEphemeralNameLocked()
 		sess, err := d.createSessionLocked(name, true, h.Cwd, sz)
-		d.mu.Unlock()
 		if err != nil {
+			d.mu.Unlock()
 			return nil, nil, err
 		}
-		return sess, d.attachClient(sess, tr, sz), nil
+		ac, old := d.attachClient(sess, tr, sz)
+		d.mu.Unlock()
+		d.detachReplacedClient(old)
+		return sess, ac, nil
 	case ports.IntentNew:
 		if h.Name == "" {
 			return nil, nil, &protoErr{ports.ErrNameTaken, "session name already in use: " + h.Name}
 		}
 		sess, err := d.createSessionLocked(h.Name, false, h.Cwd, sz)
-		d.mu.Unlock()
 		if err != nil {
+			d.mu.Unlock()
 			return nil, nil, err
 		}
-		return sess, d.attachClient(sess, tr, sz), nil
+		ac, old := d.attachClient(sess, tr, sz)
+		d.mu.Unlock()
+		d.detachReplacedClient(old)
+		return sess, ac, nil
 	case ports.IntentAttach:
 		sess := d.findByNameLocked(h.Name)
-		d.mu.Unlock()
 		if sess == nil {
+			d.mu.Unlock()
 			return nil, nil, &protoErr{ports.ErrNoSuchSession, "no such session: " + h.Name}
 		}
-		return sess, d.attachClient(sess, tr, sz), nil
+		ac, old := d.attachClient(sess, tr, sz)
+		d.mu.Unlock()
+		d.detachReplacedClient(old)
+		return sess, ac, nil
 	default:
 		d.mu.Unlock()

M internal/usecase/daemon/overlay.go +2 -2


@@ -75,7 +75,7 @@ func routeOverlayBytes(data []byte, pending *[]byte, ev overlayEvents) {
75	75		}
76	76
77	77		func routeOverlayEscape(data []byte, ev overlayEvents) (int, bool) {
78		-	if len(data) >= 3 && data[1] == '[' {
	78	+	if len(data) >= 3 && (data[1] == '[' \|\| data[1] == 'O') {
79	79		switch data[2] {
80	80		case 'A':
81	81		call(ev.up)

@@ -89,7 +89,7 @@ func routeOverlayEscape(data []byte, ev overlayEvents) (int, bool) {
89	89		}
90	90
91	91		func isOverlayEscapePrefix(data []byte) bool {
92		-	return len(data) == 2 && data[0] == 0x1b && data[1] == '['
	92	+	return len(data) == 2 && data[0] == 0x1b && (data[1] == '[' \|\| data[1] == 'O')
93	93		}
94	94
95	95		func call(fn func()) {


@@ -75,7 +75,7 @@ func routeOverlayBytes(data []byte, pending *[]byte, ev overlayEvents) {
75		}	75		}
76			76
77		func routeOverlayEscape(data []byte, ev overlayEvents) (int, bool) {	77		func routeOverlayEscape(data []byte, ev overlayEvents) (int, bool) {
78	-	if len(data) >= 3 && data[1] == '[' {	78	+	if len(data) >= 3 && (data[1] == '[' \|\| data[1] == 'O') {
79		switch data[2] {	79		switch data[2] {
80		case 'A':	80		case 'A':
81		call(ev.up)	81		call(ev.up)

@@ -89,7 +89,7 @@ func routeOverlayEscape(data []byte, ev overlayEvents) (int, bool) {
89		}	89		}
90			90
91		func isOverlayEscapePrefix(data []byte) bool {	91		func isOverlayEscapePrefix(data []byte) bool {
92	-	return len(data) == 2 && data[0] == 0x1b && data[1] == '['	92	+	return len(data) == 2 && data[0] == 0x1b && (data[1] == '[' \|\| data[1] == 'O')
93		}	93		}
94			94
95		func call(fn func()) {	95		func call(fn func()) {

M internal/usecase/daemon/overlay_test.go +17 -0


@@ -39,6 +39,23 @@ func TestRouteOverlayBytesHandlesSplitArrowsAndCtrlNavigation(t *testing.T) {
39	39		require.Equal(t, 2, down)
40	40		}
41	41
	42	+	func TestRouteOverlayBytesHandlesSS3Arrows(t *testing.T) {
	43	+	var pending []byte
	44	+	var up, down int
	45	+	ev := overlayEvents{
	46	+	up: func() { up++ },
	47	+	down: func() { down++ },
	48	+	}
	49	+
	50	+	routeOverlayBytes([]byte("\x1bO"), &pending, ev)
	51	+	require.Equal(t, []byte("\x1bO"), pending)
	52	+	routeOverlayBytes([]byte("A\x1bOB"), &pending, ev)
	53	+
	54	+	require.Equal(t, 1, up)
	55	+	require.Equal(t, 1, down)
	56	+	require.Empty(t, pending)
	57	+	}
	58	+
42	59		func TestRouteOverlayBytesHandlesEditingSubmitAndCancel(t *testing.T) {
43	60		var pending []byte
44	61		var backspace, enter, cancel int


@@ -39,6 +39,23 @@ func TestRouteOverlayBytesHandlesSplitArrowsAndCtrlNavigation(t *testing.T) {
39	require.Equal(t, 2, down)	39		require.Equal(t, 2, down)
40	}	40		}
41		41
		42	+	func TestRouteOverlayBytesHandlesSS3Arrows(t *testing.T) {
		43	+	var pending []byte
		44	+	var up, down int
		45	+	ev := overlayEvents{
		46	+	up: func() { up++ },
		47	+	down: func() { down++ },
		48	+	}
		49	+
		50	+	routeOverlayBytes([]byte("\x1bO"), &pending, ev)
		51	+	require.Equal(t, []byte("\x1bO"), pending)
		52	+	routeOverlayBytes([]byte("A\x1bOB"), &pending, ev)
		53	+
		54	+	require.Equal(t, 1, up)
		55	+	require.Equal(t, 1, down)
		56	+	require.Empty(t, pending)
		57	+	}
		58	+
42	func TestRouteOverlayBytesHandlesEditingSubmitAndCancel(t *testing.T) {	59		func TestRouteOverlayBytesHandlesEditingSubmitAndCancel(t *testing.T) {
43	var pending []byte	60		var pending []byte
44	var backspace, enter, cancel int	61		var backspace, enter, cancel int

M internal/usecase/daemon/picker.go +2 -2


@@ -179,7 +179,7 @@ func (d Daemon) stopPickerPendingTimerLocked(ac attachedClient) {
179	179		}
180	180
181	181		func routePickerEscape(m *picker.Model, data []byte) (int, bool) {
182		-	if len(data) >= 3 && data[1] == '[' {
	182	+	if len(data) >= 3 && (data[1] == '[' \|\| data[1] == 'O') {
183	183		switch data[2] {
184	184		case 'A':
185	185		m.Up()

@@ -193,7 +193,7 @@ func routePickerEscape(m *picker.Model, data []byte) (int, bool) {
193	193		}
194	194
195	195		func isPickerEscapePrefix(data []byte) bool {
196		-	return len(data) == 2 && data[0] == 0x1b && data[1] == '['
	196	+	return len(data) == 2 && data[0] == 0x1b && (data[1] == '[' \|\| data[1] == 'O')
197	197		}
198	198
199	199		func (d Daemon) registerPreviewForSelection(ac attachedClient) {


@@ -179,7 +179,7 @@ func (d Daemon) stopPickerPendingTimerLocked(ac attachedClient) {
179		}	179		}
180			180
181		func routePickerEscape(m *picker.Model, data []byte) (int, bool) {	181		func routePickerEscape(m *picker.Model, data []byte) (int, bool) {
182	-	if len(data) >= 3 && data[1] == '[' {	182	+	if len(data) >= 3 && (data[1] == '[' \|\| data[1] == 'O') {
183		switch data[2] {	183		switch data[2] {
184		case 'A':	184		case 'A':
185		m.Up()	185		m.Up()

@@ -193,7 +193,7 @@ func routePickerEscape(m *picker.Model, data []byte) (int, bool) {
193		}	193		}
194			194
195		func isPickerEscapePrefix(data []byte) bool {	195		func isPickerEscapePrefix(data []byte) bool {
196	-	return len(data) == 2 && data[0] == 0x1b && data[1] == '['	196	+	return len(data) == 2 && data[0] == 0x1b && (data[1] == '[' \|\| data[1] == 'O')
197		}	197		}
198			198
199		func (d Daemon) registerPreviewForSelection(ac attachedClient) {	199		func (d Daemon) registerPreviewForSelection(ac attachedClient) {

M internal/usecase/daemon/picker_test.go +1 -0


@@ -69,6 +69,7 @@ func TestPickerSplitArrowNavigatesWithoutExiting(t *testing.T) {
69	69		{name: "escape then down arrow", input: [][]byte{[]byte("\x1b"), []byte("[B")}, wantActive: 1},
70	70		{name: "escape then up arrow", input: [][]byte{[]byte("j"), []byte("\x1b"), []byte("[A")}, wantActive: 0},
71	71		{name: "split down arrow", input: [][]byte{[]byte("\x1b["), []byte("B")}, wantActive: 1},
	72	+	{name: "split SS3 down arrow", input: [][]byte{[]byte("\x1bO"), []byte("B")}, wantActive: 1},
72	73		}
73	74		for _, tc := range cases {
74	75		t.Run(tc.name, func(t *testing.T) {


@@ -69,6 +69,7 @@ func TestPickerSplitArrowNavigatesWithoutExiting(t *testing.T) {
69	{name: "escape then down arrow", input: [][]byte{[]byte("\x1b"), []byte("[B")}, wantActive: 1},	69		{name: "escape then down arrow", input: [][]byte{[]byte("\x1b"), []byte("[B")}, wantActive: 1},
70	{name: "escape then up arrow", input: [][]byte{[]byte("j"), []byte("\x1b"), []byte("[A")}, wantActive: 0},	70		{name: "escape then up arrow", input: [][]byte{[]byte("j"), []byte("\x1b"), []byte("[A")}, wantActive: 0},
71	{name: "split down arrow", input: [][]byte{[]byte("\x1b["), []byte("B")}, wantActive: 1},	71		{name: "split down arrow", input: [][]byte{[]byte("\x1b["), []byte("B")}, wantActive: 1},
		72	+	{name: "split SS3 down arrow", input: [][]byte{[]byte("\x1bO"), []byte("B")}, wantActive: 1},
72	}	73		}
73	for _, tc := range cases {	74		for _, tc := range cases {
74	t.Run(tc.name, func(t *testing.T) {	75		t.Run(tc.name, func(t *testing.T) {