fix(daemon): cancel tab opens with session

commit: 4c50de83dda29e5c97d67fd16c63324297d2d662
author: brice · 3mo ago · 2026-07-12 14:20 UTC
parent: 5f064ea
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

2 files · +29 -21

M internal/usecase/daemon/session.go +1 -1


@@ -272,7 +272,7 @@ func (d Daemon) createTab(sess session, sz domain.Size) error {
272	272		if err != nil {
273	273		return err
274	274		}
275		-	pty, err := d.ptys.Open(d.serveCtx, d.shell, d.shellArgs, d.childEnv(name, tabStableID, paneStableID, term), cwd, tbSize)
	275	+	pty, err := d.ptys.Open(sess.ctx, d.shell, d.shellArgs, d.childEnv(name, tabStableID, paneStableID, term), cwd, tbSize)
276	276		if err != nil {
277	277		d.log.Warn("pty spawn failed", "err", err, "session", name, "kind", "tab")
278	278		return fmt.Errorf("daemon: spawning tab for session %q: %w", name, err)


@@ -272,7 +272,7 @@ func (d Daemon) createTab(sess session, sz domain.Size) error {
272		if err != nil {	272		if err != nil {
273		return err	273		return err
274		}	274		}
275	-	pty, err := d.ptys.Open(d.serveCtx, d.shell, d.shellArgs, d.childEnv(name, tabStableID, paneStableID, term), cwd, tbSize)	275	+	pty, err := d.ptys.Open(sess.ctx, d.shell, d.shellArgs, d.childEnv(name, tabStableID, paneStableID, term), cwd, tbSize)
276		if err != nil {	276		if err != nil {
277		d.log.Warn("pty spawn failed", "err", err, "session", name, "kind", "tab")	277		d.log.Warn("pty spawn failed", "err", err, "session", name, "kind", "tab")
278		return fmt.Errorf("daemon: spawning tab for session %q: %w", name, err)	278		return fmt.Errorf("daemon: spawning tab for session %q: %w", name, err)

M internal/usecase/daemon/session_test.go +28 -20


@@ -21,6 +21,11 @@ import (
21	21		)
22	22
23	23		// --- test doubles -----------------------------------------------------------
	24	+	func expectFloatingPrewarmOpen(factory *portsmocks.MockPTYFactory, normalSize domain.Size, floating ports.PTY) {
	25	+	factory.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
	26	+	return got != normalSize && got.Valid()
	27	+	})).Return(floating, nil).Maybe()
	28	+	}
24	29
25	30		// stubClock returns timers whose channel never fires, so a scheduler under it
26	31		// blocks in its debounce loop until the session context is cancelled. Used by

@@ -44,9 +49,7 @@ func TestRoutePropagatesHelloCwdAndTabsInheritIt(t *testing.T) {
44	49		},
45	50		).Twice()
46	51		floating := newQuietPTY()
47		-	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
48		-	return got != normalSize && got.Valid()
49		-	})).Return(floating, nil).Maybe()
	52	+	expectFloatingPrewarmOpen(f, normalSize, floating)
50	53
51	54		d := newTestDaemon(t, f, stubClock{})
52	55		tr := portsmocks.NewMockTransport(t)

@@ -199,7 +202,7 @@ func TestCreateTabClosesPTYIfSessionKilledDuringOpen(t *testing.T) {
199	202		p1, release1 := newBlockingPTY(t)
200	203		defer release1()
201	204		p2 := portsmocks.NewMockPTY(t)
202		-	opened := make(chan struct{})
	205	+	openCtx := make(chan context.Context, 1)
203	206		releaseOpen := make(chan struct{})
204	207		closed := make(chan struct{})
205	208		p2.EXPECT().Close().RunAndReturn(func() error {

@@ -212,13 +215,14 @@ func TestCreateTabClosesPTYIfSessionKilledDuringOpen(t *testing.T) {
212	215		normalSize := domain.Size{Cols: 80, Rows: 22}
213	216		f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, normalSize).Return(p1, nil).Once()
214	217		floating := newQuietPTY()
215		-	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
216		-	return got != normalSize && got.Valid()
217		-	})).Return(floating, nil).Maybe()
	218	+	expectFloatingPrewarmOpen(f, normalSize, floating)
218	219		f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, normalSize).RunAndReturn(
219		-	func(context.Context, string, []string, []string, string, domain.Size) (ports.PTY, error) {
220		-	close(opened)
221		-	<-releaseOpen
	220	+	func(ctx context.Context, _ string, _ []string, _ []string, _ string, _ domain.Size) (ports.PTY, error) {
	221	+	openCtx <- ctx
	222	+	select {
	223	+	case <-ctx.Done():
	224	+	case <-releaseOpen:
	225	+	}
222	226		return p2, nil
223	227		},
224	228		).Once()

@@ -234,10 +238,20 @@ func TestCreateTabClosesPTYIfSessionKilledDuringOpen(t *testing.T) {
234	238
235	239		errCh := make(chan error, 1)
236	240		go func() { errCh <- d.createTab(sess, domain.Size{Cols: 80, Rows: 24}) }()
237		-	<-opened
	241	+	ctx := <-openCtx
238	242		_ = d.killSession(sess, ports.ReasonSessionKilled, false)
	243	+
	244	+	cancelled := true
	245	+	select {
	246	+	case <-ctx.Done():
	247	+	case <-time.After(time.Second):
	248	+	cancelled = false
	249	+	}
239	250		close(releaseOpen)
240	251
	252	+	if !cancelled {
	253	+	t.Error("killSession did not cancel the context passed to PTYFactory.Open")
	254	+	}
241	255		require.Error(t, <-errCh)
242	256		select {
243	257		case <-closed:

@@ -1276,9 +1290,7 @@ func TestAttachUpdatesFutureChildEnvTrueColor(t *testing.T) {
1276	1290		},
1277	1291		).Twice()
1278	1292		floating := newQuietPTY()
1279		-	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
1280		-	return got != normalSize && got.Valid()
1281		-	})).Return(floating, nil).Maybe()
	1293	+	expectFloatingPrewarmOpen(f, normalSize, floating)
1282	1294
1283	1295		d := newTestDaemon(t, f, stubClock{})
1284	1296		d.baseEnv = []string{"KEEP=1", "COLORTERM=old"}

@@ -1321,9 +1333,7 @@ func TestLiveAttachUpdatesFutureChildEnvTrueColor(t *testing.T) {
1321	1333		},
1322	1334		).Twice()
1323	1335		floating := newQuietPTY()
1324		-	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
1325		-	return got != normalSize && got.Valid()
1326		-	})).Return(floating, nil).Maybe()
	1336	+	expectFloatingPrewarmOpen(f, normalSize, floating)
1327	1337
1328	1338		d := newTestDaemon(t, f, stubClock{})
1329	1339		tr1 := portsmocks.NewMockTransport(t)

@@ -1368,9 +1378,7 @@ func TestCreateSessionAndSwitchInheritsTerminalEnv(t *testing.T) {
1368	1378		},
1369	1379		).Twice()
1370	1380		floating := newQuietPTY()
1371		-	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
1372		-	return got != normalSize && got.Valid()
1373		-	})).Return(floating, nil).Maybe()
	1381	+	expectFloatingPrewarmOpen(f, normalSize, floating)
1374	1382		d := newTestDaemon(t, f, stubClock{})
1375	1383		tr := portsmocks.NewMockTransport(t)
1376	1384		tr.EXPECT().Send(mock.Anything).Return(nil).Maybe()

 )
 // --- test doubles -----------------------------------------------------------
+func expectFloatingPrewarmOpen(factory *portsmocks.MockPTYFactory, normalSize domain.Size, floating ports.PTY) {
+	factory.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
+		return got != normalSize && got.Valid()
+	})).Return(floating, nil).Maybe()
+}
 // stubClock returns timers whose channel never fires, so a scheduler under it
 // blocks in its debounce loop until the session context is cancelled. Used by
 		},
 	).Twice()
 	floating := newQuietPTY()
-	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
+	expectFloatingPrewarmOpen(f, normalSize, floating)
-		return got != normalSize && got.Valid()
-	})).Return(floating, nil).Maybe()
 	d := newTestDaemon(t, f, stubClock{})
 	tr := portsmocks.NewMockTransport(t)
 	p1, release1 := newBlockingPTY(t)
 	defer release1()
 	p2 := portsmocks.NewMockPTY(t)
-	opened := make(chan struct{})
+	openCtx := make(chan context.Context, 1)
 	releaseOpen := make(chan struct{})
 	closed := make(chan struct{})
 	p2.EXPECT().Close().RunAndReturn(func() error {
 	normalSize := domain.Size{Cols: 80, Rows: 22}
 	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, normalSize).Return(p1, nil).Once()
 	floating := newQuietPTY()
-	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
+	expectFloatingPrewarmOpen(f, normalSize, floating)
-		return got != normalSize && got.Valid()
-	})).Return(floating, nil).Maybe()
 	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, normalSize).RunAndReturn(
-		func(context.Context, string, []string, []string, string, domain.Size) (ports.PTY, error) {
+		func(ctx context.Context, _ string, _ []string, _ []string, _ string, _ domain.Size) (ports.PTY, error) {
-			close(opened)
+			openCtx <- ctx
-			<-releaseOpen
+			select {
+			case <-ctx.Done():
+			case <-releaseOpen:
+			}
 			return p2, nil
 		},
 	).Once()
 	errCh := make(chan error, 1)
 	go func() { errCh <- d.createTab(sess, domain.Size{Cols: 80, Rows: 24}) }()
-	<-opened
+	ctx := <-openCtx
 	_ = d.killSession(sess, ports.ReasonSessionKilled, false)
+	cancelled := true
+	select {
+	case <-ctx.Done():
+	case <-time.After(time.Second):
+		cancelled = false
+	}
 	close(releaseOpen)
+	if !cancelled {
+		t.Error("killSession did not cancel the context passed to PTYFactory.Open")
+	}
 	require.Error(t, <-errCh)
 	select {
 	case <-closed:
 		},
 	).Twice()
 	floating := newQuietPTY()
-	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
+	expectFloatingPrewarmOpen(f, normalSize, floating)
-		return got != normalSize && got.Valid()
-	})).Return(floating, nil).Maybe()
 	d := newTestDaemon(t, f, stubClock{})
 	d.baseEnv = []string{"KEEP=1", "COLORTERM=old"}
 		},
 	).Twice()
 	floating := newQuietPTY()
-	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
+	expectFloatingPrewarmOpen(f, normalSize, floating)
-		return got != normalSize && got.Valid()
-	})).Return(floating, nil).Maybe()
 	d := newTestDaemon(t, f, stubClock{})
 	tr1 := portsmocks.NewMockTransport(t)
 		},
 	).Twice()
 	floating := newQuietPTY()
-	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
+	expectFloatingPrewarmOpen(f, normalSize, floating)
-		return got != normalSize && got.Valid()
-	})).Return(floating, nil).Maybe()
 	d := newTestDaemon(t, f, stubClock{})
 	tr := portsmocks.NewMockTransport(t)
 	tr.EXPECT().Send(mock.Anything).Return(nil).Maybe()