detached 06d1abf

Merge pull request #85 from bnema/fix/runtime-resilience

fix(runtime): harden lifecycle and validation boundaries
commit
06d1abf4e26f22c0943a70656cbc9a8d543f98fd
author
bnema · · 2026-07-14 03:02 UTC
committer
GitHub · · 2026-07-14 03:02 UTC
merge of
b8c07f6 + 3815014 · diff against the first parent
signature
GPG UnverifiedB5690EEEBB952194 The key is unknown
46 files · +1379 -453
Unified
UnifiedSplit
M cmd/vev-perf-harness/launcher.go +20 -14
@@ -281,26 +281,38 @@ func (l *cliLauncher) preparePeer(m processMapping, role roleCommand) (launchedP
281281 runtimeDir := l.runtimes[runDir]
282282 l.mu.Unlock()
283283 peerOwnsRuntime := runtimeDir == ""
284+ fail := func(err error) error {
285+ if peerOwnsRuntime {
286+ return errors.Join(err, l.releaseRuntime(runDir))
287+ }
288+ return err
289+ }
284290 if peerOwnsRuntime {
285291 var err error
286292 runtimeDir, err = l.runtimeDir(runDir)
287293 if err != nil {
288- return nil, err
294+ return nil, fail(err)
289295 }
290296 }
291297 if err := safedir.EnsurePrivate(runDir); err != nil {
292- return nil, err
298+ return nil, fail(err)
293299 }
294300 if err := safedir.EnsurePrivate(runtimeDir); err != nil {
295- return nil, err
301+ return nil, fail(err)
296302 }
297303 if err := safedir.EnsurePrivate(filepath.Join(runDir, "state")); err != nil {
298- return nil, err
304+ return nil, fail(err)
299305 }
300306 if len(role.Args) != 2 || (role.Args[0] != "_stdio" && role.Args[0] != "_udp-proxy") {
301- return nil, fmt.Errorf("unsupported public peer command %q", role.Args)
307+ return nil, fail(fmt.Errorf("unsupported public peer command %q", role.Args))
302308 }
303309 route := peerRoute{mapping: m, command: role}
310+ closeNetem := func(err error) error {
311+ if route.netem != nil {
312+ err = errors.Join(err, route.netem.Close())
313+ }
314+ return fail(err)
315+ }
304316 if m.Role == "udp_peer" {
305317 route.pidPath = filepath.Join(runDir, "udp-peer.pid")
306318 factory := l.netemFactory
@@ -313,7 +325,7 @@ func (l *cliLauncher) preparePeer(m processMapping, role roleCommand) (launchedP
313325 TargetPath: filepath.Join(runDir, "udp-peer.target"),
314326 })
315327 if err != nil {
316- return nil, fmt.Errorf("start harness UDP netem: %w", err)
328+ return nil, fail(fmt.Errorf("start harness UDP netem: %w", err))
317329 }
318330 route.netem = netem
319331 }
@@ -354,10 +366,7 @@ case "$*" in
354366 esac
355367 `, m.TracePath, m.ProcessID, runtimeDir, filepath.Join(runDir, "state"), l.bin, role.Args[1], ready, stderr, target, netemPort, route.pidPath, m.Scenario, m.Run)
356368 if err := os.WriteFile(shim, []byte(body), 0o700); err != nil {
357- if route.netem != nil {
358- _ = route.netem.Close()
359- }
360- return nil, err
369+ return nil, closeNetem(err)
361370 }
362371 l.mu.Lock()
363372 if l.peers == nil {
@@ -365,10 +374,7 @@ esac
365374 }
366375 if _, exists := l.peers[runDir]; exists {
367376 l.mu.Unlock()
368- if route.netem != nil {
369- _ = route.netem.Close()
370- }
371- return nil, fmt.Errorf("duplicate transport peer for %s", runDir)
377+ return nil, closeNetem(fmt.Errorf("duplicate transport peer for %s", runDir))
372378 }
373379 l.peers[runDir] = route
374380 l.mu.Unlock()
@@ -281,26 +281,38 @@ func (l *cliLauncher) preparePeer(m processMapping, role roleCommand) (launchedP
281 runtimeDir := l.runtimes[runDir]281 runtimeDir := l.runtimes[runDir]
282 l.mu.Unlock()282 l.mu.Unlock()
283 peerOwnsRuntime := runtimeDir == ""283 peerOwnsRuntime := runtimeDir == ""
284+ fail := func(err error) error {
285+ if peerOwnsRuntime {
286+ return errors.Join(err, l.releaseRuntime(runDir))
287+ }
288+ return err
289+ }
284 if peerOwnsRuntime {290 if peerOwnsRuntime {
285 var err error291 var err error
286 runtimeDir, err = l.runtimeDir(runDir)292 runtimeDir, err = l.runtimeDir(runDir)
287 if err != nil {293 if err != nil {
288- return nil, err294+ return nil, fail(err)
289 }295 }
290 }296 }
291 if err := safedir.EnsurePrivate(runDir); err != nil {297 if err := safedir.EnsurePrivate(runDir); err != nil {
292- return nil, err298+ return nil, fail(err)
293 }299 }
294 if err := safedir.EnsurePrivate(runtimeDir); err != nil {300 if err := safedir.EnsurePrivate(runtimeDir); err != nil {
295- return nil, err301+ return nil, fail(err)
296 }302 }
297 if err := safedir.EnsurePrivate(filepath.Join(runDir, "state")); err != nil {303 if err := safedir.EnsurePrivate(filepath.Join(runDir, "state")); err != nil {
298- return nil, err304+ return nil, fail(err)
299 }305 }
300 if len(role.Args) != 2 || (role.Args[0] != "_stdio" && role.Args[0] != "_udp-proxy") {306 if len(role.Args) != 2 || (role.Args[0] != "_stdio" && role.Args[0] != "_udp-proxy") {
301- return nil, fmt.Errorf("unsupported public peer command %q", role.Args)307+ return nil, fail(fmt.Errorf("unsupported public peer command %q", role.Args))
302 }308 }
303 route := peerRoute{mapping: m, command: role}309 route := peerRoute{mapping: m, command: role}
310+ closeNetem := func(err error) error {
311+ if route.netem != nil {
312+ err = errors.Join(err, route.netem.Close())
313+ }
314+ return fail(err)
315+ }
304 if m.Role == "udp_peer" {316 if m.Role == "udp_peer" {
305 route.pidPath = filepath.Join(runDir, "udp-peer.pid")317 route.pidPath = filepath.Join(runDir, "udp-peer.pid")
306 factory := l.netemFactory318 factory := l.netemFactory
@@ -313,7 +325,7 @@ func (l *cliLauncher) preparePeer(m processMapping, role roleCommand) (launchedP
313 TargetPath: filepath.Join(runDir, "udp-peer.target"),325 TargetPath: filepath.Join(runDir, "udp-peer.target"),
314 })326 })
315 if err != nil {327 if err != nil {
316- return nil, fmt.Errorf("start harness UDP netem: %w", err)328+ return nil, fail(fmt.Errorf("start harness UDP netem: %w", err))
317 }329 }
318 route.netem = netem330 route.netem = netem
319 }331 }
@@ -354,10 +366,7 @@ case "$*" in
354 esac366 esac
355 `, m.TracePath, m.ProcessID, runtimeDir, filepath.Join(runDir, "state"), l.bin, role.Args[1], ready, stderr, target, netemPort, route.pidPath, m.Scenario, m.Run)367 `, m.TracePath, m.ProcessID, runtimeDir, filepath.Join(runDir, "state"), l.bin, role.Args[1], ready, stderr, target, netemPort, route.pidPath, m.Scenario, m.Run)
356 if err := os.WriteFile(shim, []byte(body), 0o700); err != nil {368 if err := os.WriteFile(shim, []byte(body), 0o700); err != nil {
357- if route.netem != nil {369+ return nil, closeNetem(err)
358- _ = route.netem.Close()
359- }
360- return nil, err
361 }370 }
362 l.mu.Lock()371 l.mu.Lock()
363 if l.peers == nil {372 if l.peers == nil {
@@ -365,10 +374,7 @@ esac
365 }374 }
366 if _, exists := l.peers[runDir]; exists {375 if _, exists := l.peers[runDir]; exists {
367 l.mu.Unlock()376 l.mu.Unlock()
368- if route.netem != nil {377+ return nil, closeNetem(fmt.Errorf("duplicate transport peer for %s", runDir))
369- _ = route.netem.Close()
370- }
371- return nil, fmt.Errorf("duplicate transport peer for %s", runDir)
372 }378 }
373 l.peers[runDir] = route379 l.peers[runDir] = route
374 l.mu.Unlock()380 l.mu.Unlock()
M cmd/vev-perf-harness/launcher_test.go +34 -4
@@ -2,6 +2,7 @@ package main
22
33 import (
44 "bytes"
5+ "context"
56 "fmt"
67 "net"
78 "os"
@@ -10,6 +11,8 @@ import (
1011 "strings"
1112 "testing"
1213 "time"
14+
15+ "github.com/bnema/vev/pkg/safedir"
1316 )
1417
1518 func TestCLIProcessCloseIsBoundedWhenPublicRoleDoesNotExit(t *testing.T) {
@@ -32,11 +35,16 @@ func TestCLIProcessCloseIsBoundedWhenPublicRoleDoesNotExit(t *testing.T) {
3235
3336 func TestCLIProcessWaitReadyRequiresDaemonSocket(t *testing.T) {
3437 path := filepath.Join(t.TempDir(), "daemon.sock")
35- listener, err := net.Listen("unix", path)
38+ var lc net.ListenConfig
39+ listener, err := lc.Listen(context.Background(), "unix", path)
3640 if err != nil {
3741 t.Fatal(err)
3842 }
39- defer listener.Close()
43+ t.Cleanup(func() {
44+ if err := listener.Close(); err != nil {
45+ t.Error(err)
46+ }
47+ })
4048 if err := (&cliProcess{readyPath: path, waitErr: make(chan error, 1)}).WaitReady(); err != nil {
4149 t.Fatal(err)
4250 }
@@ -55,7 +63,7 @@ func TestCLILauncherIsolatesRoleXDGEnvironmentAcrossRepetitions(t *testing.T) {
5563 launcher := &cliLauncher{bin: bin}
5664 for run := 1; run <= 3; run++ {
5765 runDir := filepath.Join(t.TempDir(), fmt.Sprintf("run-%d", run))
58- if err := os.MkdirAll(runDir, 0o700); err != nil {
66+ if err := safedir.EnsurePrivate(runDir); err != nil {
5967 t.Fatal(err)
6068 }
6169 roles := []string{"daemon", "client"}
@@ -95,6 +103,28 @@ func TestCLILauncherIsolatesRoleXDGEnvironmentAcrossRepetitions(t *testing.T) {
95103 }
96104 }
97105
106+func TestPreparePeerReleasesOwnedRuntimeOnPreparationFailure(t *testing.T) {
107+ t.Setenv("TMPDIR", t.TempDir())
108+ runDir := filepath.Join(t.TempDir(), "run")
109+ if err := safedir.EnsurePrivate(runDir); err != nil {
110+ t.Fatal(err)
111+ }
112+ launcher := &cliLauncher{}
113+ _, err := launcher.preparePeer(processMapping{
114+ Role: "ssh_stdio_peer",
115+ TracePath: filepath.Join(runDir, "peer.jsonl"),
116+ }, roleCommand{Args: []string{"invalid"}})
117+ if err == nil || !strings.Contains(err.Error(), "unsupported public peer command") {
118+ t.Fatalf("preparePeer error = %v", err)
119+ }
120+ if runtimeDir := launcher.runtimes[runDir]; runtimeDir != "" {
121+ t.Fatalf("failed peer preparation leaked runtime directory %q", runtimeDir)
122+ }
123+ if paths, globErr := filepath.Glob(filepath.Join(os.TempDir(), "vev-harness-runtime-*")); globErr != nil || len(paths) != 0 {
124+ t.Fatalf("failed peer preparation left runtime directories %q: %v", paths, globErr)
125+ }
126+}
127+
98128 func TestCLIProcessCloseUsesPublicDaemonShutdown(t *testing.T) {
99129 wait := make(chan error, 1)
100130 wait <- nil
@@ -153,7 +183,7 @@ func TestHarnessCanonicalLocalSmokeRealRoles(t *testing.T) {
153183 if err != nil {
154184 t.Fatal(err)
155185 }
156- defer raw.Close()
186+ closeTestFile(t, raw)
157187 h := defaultHarness()
158188 h.clock = &fakeClock{} // injected duration keeps the public 30s contract bounded in test
159189 h.launcher = &cliLauncher{bin: bin}
@@ -2,6 +2,7 @@ package main
2 2
3 import (3 import (
4 "bytes"4 "bytes"
5+ "context"
5 "fmt"6 "fmt"
6 "net"7 "net"
7 "os"8 "os"
@@ -10,6 +11,8 @@ import (
10 "strings"11 "strings"
11 "testing"12 "testing"
12 "time"13 "time"
14+
15+ "github.com/bnema/vev/pkg/safedir"
13 )16 )
14 17
15 func TestCLIProcessCloseIsBoundedWhenPublicRoleDoesNotExit(t *testing.T) {18 func TestCLIProcessCloseIsBoundedWhenPublicRoleDoesNotExit(t *testing.T) {
@@ -32,11 +35,16 @@ func TestCLIProcessCloseIsBoundedWhenPublicRoleDoesNotExit(t *testing.T) {
32 35
33 func TestCLIProcessWaitReadyRequiresDaemonSocket(t *testing.T) {36 func TestCLIProcessWaitReadyRequiresDaemonSocket(t *testing.T) {
34 path := filepath.Join(t.TempDir(), "daemon.sock")37 path := filepath.Join(t.TempDir(), "daemon.sock")
35- listener, err := net.Listen("unix", path)38+ var lc net.ListenConfig
39+ listener, err := lc.Listen(context.Background(), "unix", path)
36 if err != nil {40 if err != nil {
37 t.Fatal(err)41 t.Fatal(err)
38 }42 }
39- defer listener.Close()43+ t.Cleanup(func() {
44+ if err := listener.Close(); err != nil {
45+ t.Error(err)
46+ }
47+ })
40 if err := (&cliProcess{readyPath: path, waitErr: make(chan error, 1)}).WaitReady(); err != nil {48 if err := (&cliProcess{readyPath: path, waitErr: make(chan error, 1)}).WaitReady(); err != nil {
41 t.Fatal(err)49 t.Fatal(err)
42 }50 }
@@ -55,7 +63,7 @@ func TestCLILauncherIsolatesRoleXDGEnvironmentAcrossRepetitions(t *testing.T) {
55 launcher := &cliLauncher{bin: bin}63 launcher := &cliLauncher{bin: bin}
56 for run := 1; run <= 3; run++ {64 for run := 1; run <= 3; run++ {
57 runDir := filepath.Join(t.TempDir(), fmt.Sprintf("run-%d", run))65 runDir := filepath.Join(t.TempDir(), fmt.Sprintf("run-%d", run))
58- if err := os.MkdirAll(runDir, 0o700); err != nil {66+ if err := safedir.EnsurePrivate(runDir); err != nil {
59 t.Fatal(err)67 t.Fatal(err)
60 }68 }
61 roles := []string{"daemon", "client"}69 roles := []string{"daemon", "client"}
@@ -95,6 +103,28 @@ func TestCLILauncherIsolatesRoleXDGEnvironmentAcrossRepetitions(t *testing.T) {
95 }103 }
96 }104 }
97 105
106+func TestPreparePeerReleasesOwnedRuntimeOnPreparationFailure(t *testing.T) {
107+ t.Setenv("TMPDIR", t.TempDir())
108+ runDir := filepath.Join(t.TempDir(), "run")
109+ if err := safedir.EnsurePrivate(runDir); err != nil {
110+ t.Fatal(err)
111+ }
112+ launcher := &cliLauncher{}
113+ _, err := launcher.preparePeer(processMapping{
114+ Role: "ssh_stdio_peer",
115+ TracePath: filepath.Join(runDir, "peer.jsonl"),
116+ }, roleCommand{Args: []string{"invalid"}})
117+ if err == nil || !strings.Contains(err.Error(), "unsupported public peer command") {
118+ t.Fatalf("preparePeer error = %v", err)
119+ }
120+ if runtimeDir := launcher.runtimes[runDir]; runtimeDir != "" {
121+ t.Fatalf("failed peer preparation leaked runtime directory %q", runtimeDir)
122+ }
123+ if paths, globErr := filepath.Glob(filepath.Join(os.TempDir(), "vev-harness-runtime-*")); globErr != nil || len(paths) != 0 {
124+ t.Fatalf("failed peer preparation left runtime directories %q: %v", paths, globErr)
125+ }
126+}
127+
98 func TestCLIProcessCloseUsesPublicDaemonShutdown(t *testing.T) {128 func TestCLIProcessCloseUsesPublicDaemonShutdown(t *testing.T) {
99 wait := make(chan error, 1)129 wait := make(chan error, 1)
100 wait <- nil130 wait <- nil
@@ -153,7 +183,7 @@ func TestHarnessCanonicalLocalSmokeRealRoles(t *testing.T) {
153 if err != nil {183 if err != nil {
154 t.Fatal(err)184 t.Fatal(err)
155 }185 }
156- defer raw.Close()186+ closeTestFile(t, raw)
157 h := defaultHarness()187 h := defaultHarness()
158 h.clock = &fakeClock{} // injected duration keeps the public 30s contract bounded in test188 h.clock = &fakeClock{} // injected duration keeps the public 30s contract bounded in test
159 h.launcher = &cliLauncher{bin: bin}189 h.launcher = &cliLauncher{bin: bin}
M cmd/vev-perf-harness/options_manifest.go +21 -11
@@ -87,7 +87,13 @@ func validateManifest(m manifest) error {
8787 }
8888 tops := map[string]bool{}
8989 for _, t := range m.Topologies {
90- if t.ID == "" || t.Geometry != "120x40" || t.RowsPerPane != 10000 {
90+ if t.ID == "" {
91+ return errors.New("empty topology id")
92+ }
93+ if tops[t.ID] {
94+ return fmt.Errorf("duplicate topology id %q", t.ID)
95+ }
96+ if t.Geometry != "120x40" || t.RowsPerPane != 10000 {
9197 return fmt.Errorf("invalid topology %q", t.ID)
9298 }
9399 tops[t.ID] = true
@@ -98,7 +104,16 @@ func validateManifest(m manifest) error {
98104 return fmt.Errorf("missing canonical topology %s", v)
99105 }
100106 }
101- works := set(m.Workloads)
107+ works := map[string]bool{}
108+ for _, workload := range m.Workloads {
109+ if workload == "" {
110+ return errors.New("empty workload id")
111+ }
112+ if works[workload] {
113+ return fmt.Errorf("duplicate workload id %q", workload)
114+ }
115+ works[workload] = true
116+ }
102117 for _, v := range []string{"idle", "active_output", "all_output", "inactive_output", "interactive_flood", "copy_search", "resize_sweep", "snapshot_output_resize", "attach_restore_tab_switch"} {
103118 if !works[v] {
104119 return fmt.Errorf("missing canonical workload %s", v)
@@ -107,7 +122,10 @@ func validateManifest(m manifest) error {
107122 trans := map[string]bool{}
108123 for _, t := range m.Transports {
109124 if t.ID == "" {
110- return errors.New("transport missing id")
125+ return errors.New("empty transport id")
126+ }
127+ if trans[t.ID] {
128+ return fmt.Errorf("duplicate transport id %q", t.ID)
111129 }
112130 if err := validateTransportFixture(t); err != nil {
113131 return err
@@ -211,11 +229,3 @@ func validateTransportFixture(t transport) error {
211229 }
212230 return nil
213231 }
214-
215-func set(v []string) map[string]bool {
216- r := map[string]bool{}
217- for _, x := range v {
218- r[x] = true
219- }
220- return r
221-}
@@ -87,7 +87,13 @@ func validateManifest(m manifest) error {
87 }87 }
88 tops := map[string]bool{}88 tops := map[string]bool{}
89 for _, t := range m.Topologies {89 for _, t := range m.Topologies {
90- if t.ID == "" || t.Geometry != "120x40" || t.RowsPerPane != 10000 {90+ if t.ID == "" {
91+ return errors.New("empty topology id")
92+ }
93+ if tops[t.ID] {
94+ return fmt.Errorf("duplicate topology id %q", t.ID)
95+ }
96+ if t.Geometry != "120x40" || t.RowsPerPane != 10000 {
91 return fmt.Errorf("invalid topology %q", t.ID)97 return fmt.Errorf("invalid topology %q", t.ID)
92 }98 }
93 tops[t.ID] = true99 tops[t.ID] = true
@@ -98,7 +104,16 @@ func validateManifest(m manifest) error {
98 return fmt.Errorf("missing canonical topology %s", v)104 return fmt.Errorf("missing canonical topology %s", v)
99 }105 }
100 }106 }
101- works := set(m.Workloads)107+ works := map[string]bool{}
108+ for _, workload := range m.Workloads {
109+ if workload == "" {
110+ return errors.New("empty workload id")
111+ }
112+ if works[workload] {
113+ return fmt.Errorf("duplicate workload id %q", workload)
114+ }
115+ works[workload] = true
116+ }
102 for _, v := range []string{"idle", "active_output", "all_output", "inactive_output", "interactive_flood", "copy_search", "resize_sweep", "snapshot_output_resize", "attach_restore_tab_switch"} {117 for _, v := range []string{"idle", "active_output", "all_output", "inactive_output", "interactive_flood", "copy_search", "resize_sweep", "snapshot_output_resize", "attach_restore_tab_switch"} {
103 if !works[v] {118 if !works[v] {
104 return fmt.Errorf("missing canonical workload %s", v)119 return fmt.Errorf("missing canonical workload %s", v)
@@ -107,7 +122,10 @@ func validateManifest(m manifest) error {
107 trans := map[string]bool{}122 trans := map[string]bool{}
108 for _, t := range m.Transports {123 for _, t := range m.Transports {
109 if t.ID == "" {124 if t.ID == "" {
110- return errors.New("transport missing id")125+ return errors.New("empty transport id")
126+ }
127+ if trans[t.ID] {
128+ return fmt.Errorf("duplicate transport id %q", t.ID)
111 }129 }
112 if err := validateTransportFixture(t); err != nil {130 if err := validateTransportFixture(t); err != nil {
113 return err131 return err
@@ -211,11 +229,3 @@ func validateTransportFixture(t transport) error {
211 }229 }
212 return nil230 return nil
213 }231 }
214-
215-func set(v []string) map[string]bool {
216- r := map[string]bool{}
217- for _, x := range v {
218- r[x] = true
219- }
220- return r
221-}
M cmd/vev-perf-harness/options_manifest_test.go +31 -1
@@ -119,6 +119,36 @@ func TestHarnessFlagsRejectShortMeasurements(t *testing.T) {
119119 }
120120 }
121121
122+func TestValidateManifestRejectsEmptyAndDuplicateMatrixIDs(t *testing.T) {
123+ canonical, err := readManifest(filepath.Join("..", "..", "testdata", "perf", "manifest.json"))
124+ if err != nil {
125+ t.Fatal(err)
126+ }
127+ for _, tc := range []struct {
128+ name string
129+ mutate func(*manifest)
130+ want string
131+ }{
132+ {"empty topology", func(m *manifest) { m.Topologies[0].ID = "" }, "empty topology id"},
133+ {"duplicate topology", func(m *manifest) { m.Topologies[1].ID = m.Topologies[0].ID }, "duplicate topology id"},
134+ {"empty workload", func(m *manifest) { m.Workloads[0] = "" }, "empty workload id"},
135+ {"duplicate workload", func(m *manifest) { m.Workloads[1] = m.Workloads[0] }, "duplicate workload id"},
136+ {"empty transport", func(m *manifest) { m.Transports[0].ID = "" }, "empty transport id"},
137+ {"duplicate transport", func(m *manifest) { m.Transports[1].ID = m.Transports[0].ID }, "duplicate transport id"},
138+ } {
139+ t.Run(tc.name, func(t *testing.T) {
140+ m := canonical
141+ m.Topologies = append([]topology(nil), canonical.Topologies...)
142+ m.Workloads = append([]string(nil), canonical.Workloads...)
143+ m.Transports = append([]transport(nil), canonical.Transports...)
144+ tc.mutate(&m)
145+ if err := validateManifest(m); err == nil || !strings.Contains(err.Error(), tc.want) {
146+ t.Fatalf("validateManifest() error = %v, want %q", err, tc.want)
147+ }
148+ })
149+ }
150+}
151+
122152 func TestHarnessManifestCoversCanonicalMatrix(t *testing.T) {
123153 m, err := readManifest(filepath.Join("..", "..", "testdata", "perf", "manifest.json"))
124154 if err != nil {
@@ -142,7 +172,7 @@ func TestHarnessCreatesExclusiveTraceManifestAndEvidence(t *testing.T) {
142172 if err != nil {
143173 t.Fatal(err)
144174 }
145- defer raw.Close()
175+ closeTestFile(t, raw)
146176 launcher := &fakeLauncher{}
147177 h := defaultHarness()
148178 h.clock = &fakeClock{}
@@ -119,6 +119,36 @@ func TestHarnessFlagsRejectShortMeasurements(t *testing.T) {
119 }119 }
120 }120 }
121 121
122+func TestValidateManifestRejectsEmptyAndDuplicateMatrixIDs(t *testing.T) {
123+ canonical, err := readManifest(filepath.Join("..", "..", "testdata", "perf", "manifest.json"))
124+ if err != nil {
125+ t.Fatal(err)
126+ }
127+ for _, tc := range []struct {
128+ name string
129+ mutate func(*manifest)
130+ want string
131+ }{
132+ {"empty topology", func(m *manifest) { m.Topologies[0].ID = "" }, "empty topology id"},
133+ {"duplicate topology", func(m *manifest) { m.Topologies[1].ID = m.Topologies[0].ID }, "duplicate topology id"},
134+ {"empty workload", func(m *manifest) { m.Workloads[0] = "" }, "empty workload id"},
135+ {"duplicate workload", func(m *manifest) { m.Workloads[1] = m.Workloads[0] }, "duplicate workload id"},
136+ {"empty transport", func(m *manifest) { m.Transports[0].ID = "" }, "empty transport id"},
137+ {"duplicate transport", func(m *manifest) { m.Transports[1].ID = m.Transports[0].ID }, "duplicate transport id"},
138+ } {
139+ t.Run(tc.name, func(t *testing.T) {
140+ m := canonical
141+ m.Topologies = append([]topology(nil), canonical.Topologies...)
142+ m.Workloads = append([]string(nil), canonical.Workloads...)
143+ m.Transports = append([]transport(nil), canonical.Transports...)
144+ tc.mutate(&m)
145+ if err := validateManifest(m); err == nil || !strings.Contains(err.Error(), tc.want) {
146+ t.Fatalf("validateManifest() error = %v, want %q", err, tc.want)
147+ }
148+ })
149+ }
150+}
151+
122 func TestHarnessManifestCoversCanonicalMatrix(t *testing.T) {152 func TestHarnessManifestCoversCanonicalMatrix(t *testing.T) {
123 m, err := readManifest(filepath.Join("..", "..", "testdata", "perf", "manifest.json"))153 m, err := readManifest(filepath.Join("..", "..", "testdata", "perf", "manifest.json"))
124 if err != nil {154 if err != nil {
@@ -142,7 +172,7 @@ func TestHarnessCreatesExclusiveTraceManifestAndEvidence(t *testing.T) {
142 if err != nil {172 if err != nil {
143 t.Fatal(err)173 t.Fatal(err)
144 }174 }
145- defer raw.Close()175+ closeTestFile(t, raw)
146 launcher := &fakeLauncher{}176 launcher := &fakeLauncher{}
147 h := defaultHarness()177 h := defaultHarness()
148 h.clock = &fakeClock{}178 h.clock = &fakeClock{}
M cmd/vev-perf-harness/runner_test.go +3 -1
@@ -11,6 +11,8 @@ import (
1111 "strings"
1212 "testing"
1313 "time"
14+
15+ "github.com/bnema/vev/pkg/safedir"
1416 )
1517
1618 func TestHarnessCanonicalLocalRolesAreIsolatedAcrossRepetitions(t *testing.T) {
@@ -126,7 +128,7 @@ func TestHarnessResolvesCanonicalRelativePathsBeforeRoleCWDChanges(t *testing.T)
126128 t.Fatalf("build public CLI: %v\n%s", err, output)
127129 }
128130 manifestPath := filepath.Join(workspace, "testdata", "perf", "manifest.json")
129- if err := os.MkdirAll(filepath.Dir(manifestPath), 0o700); err != nil {
131+ if err := safedir.EnsurePrivate(filepath.Dir(manifestPath)); err != nil {
130132 t.Fatal(err)
131133 }
132134 contents, err := os.ReadFile(filepath.Join(root, "testdata", "perf", "manifest.json"))
@@ -11,6 +11,8 @@ import (
11 "strings"11 "strings"
12 "testing"12 "testing"
13 "time"13 "time"
14+
15+ "github.com/bnema/vev/pkg/safedir"
14 )16 )
15 17
16 func TestHarnessCanonicalLocalRolesAreIsolatedAcrossRepetitions(t *testing.T) {18 func TestHarnessCanonicalLocalRolesAreIsolatedAcrossRepetitions(t *testing.T) {
@@ -126,7 +128,7 @@ func TestHarnessResolvesCanonicalRelativePathsBeforeRoleCWDChanges(t *testing.T)
126 t.Fatalf("build public CLI: %v\n%s", err, output)128 t.Fatalf("build public CLI: %v\n%s", err, output)
127 }129 }
128 manifestPath := filepath.Join(workspace, "testdata", "perf", "manifest.json")130 manifestPath := filepath.Join(workspace, "testdata", "perf", "manifest.json")
129- if err := os.MkdirAll(filepath.Dir(manifestPath), 0o700); err != nil {131+ if err := safedir.EnsurePrivate(filepath.Dir(manifestPath)); err != nil {
130 t.Fatal(err)132 t.Fatal(err)
131 }133 }
132 contents, err := os.ReadFile(filepath.Join(root, "testdata", "perf", "manifest.json"))134 contents, err := os.ReadFile(filepath.Join(root, "testdata", "perf", "manifest.json"))
M cmd/vev-perf-harness/udp_netem.go +3 -1
@@ -2,6 +2,7 @@ package main
22
33 import (
44 "container/heap"
5+ "context"
56 "errors"
67 "fmt"
78 "net"
@@ -85,7 +86,8 @@ func newUDPNetem(config udpNetemConfig) (udpNetem, error) {
8586 if config.RTT < 0 || config.LossPercent < 0 || config.LossPercent > 100 || config.TargetPath == "" {
8687 return nil, fmt.Errorf("invalid UDP netem fixture: rtt=%s loss=%d target=%q", config.RTT, config.LossPercent, config.TargetPath)
8788 }
88- conn, err := net.ListenPacket("udp", "127.0.0.1:0")
89+ var lc net.ListenConfig
90+ conn, err := lc.ListenPacket(context.Background(), "udp", "127.0.0.1:0")
8991 if err != nil {
9092 return nil, err
9193 }
@@ -2,6 +2,7 @@ package main
2 2
3 import (3 import (
4 "container/heap"4 "container/heap"
5+ "context"
5 "errors"6 "errors"
6 "fmt"7 "fmt"
7 "net"8 "net"
@@ -85,7 +86,8 @@ func newUDPNetem(config udpNetemConfig) (udpNetem, error) {
85 if config.RTT < 0 || config.LossPercent < 0 || config.LossPercent > 100 || config.TargetPath == "" {86 if config.RTT < 0 || config.LossPercent < 0 || config.LossPercent > 100 || config.TargetPath == "" {
86 return nil, fmt.Errorf("invalid UDP netem fixture: rtt=%s loss=%d target=%q", config.RTT, config.LossPercent, config.TargetPath)87 return nil, fmt.Errorf("invalid UDP netem fixture: rtt=%s loss=%d target=%q", config.RTT, config.LossPercent, config.TargetPath)
87 }88 }
88- conn, err := net.ListenPacket("udp", "127.0.0.1:0")89+ var lc net.ListenConfig
90+ conn, err := lc.ListenPacket(context.Background(), "udp", "127.0.0.1:0")
89 if err != nil {91 if err != nil {
90 return nil, err92 return nil, err
91 }93 }
M cmd/vev-perf-harness/udp_netem_test.go +4 -2
@@ -1,6 +1,7 @@
11 package main
22
33 import (
4+ "context"
45 "net"
56 "os"
67 "path/filepath"
@@ -68,7 +69,8 @@ func TestUDPNetemExecutesRTTAndLossFixtures(t *testing.T) {
6869 }
6970 for _, tc := range fixtures {
7071 t.Run(tc.name, func(t *testing.T) {
71- target, err := net.ListenPacket("udp", "127.0.0.1:0")
72+ var lc net.ListenConfig
73+ target, err := lc.ListenPacket(context.Background(), "udp", "127.0.0.1:0")
7274 if err != nil {
7375 t.Fatal(err)
7476 }
@@ -86,7 +88,7 @@ func TestUDPNetemExecutesRTTAndLossFixtures(t *testing.T) {
8688 t.Fatal(err)
8789 }
8890 closeUDPFixture(t, netem)
89- client, err := net.ListenPacket("udp", "127.0.0.1:0")
91+ client, err := lc.ListenPacket(context.Background(), "udp", "127.0.0.1:0")
9092 if err != nil {
9193 t.Fatal(err)
9294 }
@@ -1,6 +1,7 @@
1 package main1 package main
2 2
3 import (3 import (
4+ "context"
4 "net"5 "net"
5 "os"6 "os"
6 "path/filepath"7 "path/filepath"
@@ -68,7 +69,8 @@ func TestUDPNetemExecutesRTTAndLossFixtures(t *testing.T) {
68 }69 }
69 for _, tc := range fixtures {70 for _, tc := range fixtures {
70 t.Run(tc.name, func(t *testing.T) {71 t.Run(tc.name, func(t *testing.T) {
71- target, err := net.ListenPacket("udp", "127.0.0.1:0")72+ var lc net.ListenConfig
73+ target, err := lc.ListenPacket(context.Background(), "udp", "127.0.0.1:0")
72 if err != nil {74 if err != nil {
73 t.Fatal(err)75 t.Fatal(err)
74 }76 }
@@ -86,7 +88,7 @@ func TestUDPNetemExecutesRTTAndLossFixtures(t *testing.T) {
86 t.Fatal(err)88 t.Fatal(err)
87 }89 }
88 closeUDPFixture(t, netem)90 closeUDPFixture(t, netem)
89- client, err := net.ListenPacket("udp", "127.0.0.1:0")91+ client, err := lc.ListenPacket(context.Background(), "udp", "127.0.0.1:0")
90 if err != nil {92 if err != nil {
91 t.Fatal(err)93 t.Fatal(err)
92 }94 }
M internal/adapters/ipc/transport.go +157 -31
@@ -27,15 +27,25 @@ var ErrZeroLengthFrame = errors.New("ipc: zero-length frame")
2727 // maxFrameLen, and by Send when a payload is too large to encode.
2828 var ErrFrameTooLarge = errors.New("ipc: frame exceeds maximum length")
2929
30+// ErrBackpressure means the bounded IPC egress queue is full. Callers using
31+// SendAsync must treat it as a failed render emission rather than spawning an
32+// unbounded queue of their own.
33+var ErrBackpressure = errors.New("ipc: egress queue full")
34+
35+var errClosed = errors.New("ipc: closed")
36+
37+// sendQueueCapacity bounds frames accepted while a peer is not reading. It is
38+// deliberately small: render output can be large and must never turn a wedged
39+// peer into an unbounded memory commitment.
40+const sendQueueCapacity = 8
41+
3042 // unixTransport implements ports.Transport over a net.Conn (in practice an
3143 // AF_UNIX SOCK_STREAM connection, but any net.Conn works — this also lets
3244 // tests exercise it over net.Pipe).
3345 //
34-// unixTransport is safe for one concurrent caller of Send and one
35-// concurrent caller of Recv at the same time (i.e. a single reader
36-// goroutine and a single writer goroutine); it is not safe for concurrent
37-// calls to Send from multiple goroutines, nor Recv from multiple
38-// goroutines.
46+// unixTransport has one writer goroutine which owns every conn.Write. Send
47+// waits for that owner's wire attempt; SendAsync only performs a bounded,
48+// ordered enqueue. Recv still has one caller at a time.
3949 type Option func(*unixTransport)
4050
4151 // WithRuntimeObserver enables process-local transport marks. It accepts no
@@ -45,12 +55,23 @@ func WithRuntimeObserver(observer ports.SerializedRuntimeObserver) Option {
4555 }
4656
4757 type unixTransport struct {
48- conn net.Conn
49- observer ports.SerializedRuntimeObserver
50- operationMu sync.Mutex
51- operationCount int
52- closing bool
53- operationsDone chan struct{}
58+ conn net.Conn
59+ observer ports.SerializedRuntimeObserver
60+
61+ operationMu sync.Mutex
62+ operationCount int
63+ observedClosing bool
64+ operationsDone chan struct{}
65+
66+ // egressMu synchronizes Close with nonblocking enqueue. Once closed, no
67+ // request can be left behind after the writer drains the queue.
68+ egressMu sync.Mutex
69+ egressClosing bool
70+ egress chan sendRequest
71+ done chan struct{}
72+ writerDone chan struct{}
73+ closeOnce sync.Once
74+ closeErr error
5475
5576 // readBuf is reused across Recv calls (grow-once strategy): it grows
5677 // to fit the largest frame seen so far and is never shrunk. The Frame
@@ -62,34 +83,132 @@ type unixTransport struct {
6283 // NewTransport wraps conn as a ports.Transport speaking vev's framed
6384 // binary protocol.
6485 func NewTransport(conn net.Conn, opts ...Option) ports.Transport {
65- t := &unixTransport{conn: conn}
86+ t := &unixTransport{
87+ conn: conn,
88+ egress: make(chan sendRequest, sendQueueCapacity),
89+ done: make(chan struct{}),
90+ writerDone: make(chan struct{}),
91+ }
6692 for _, opt := range opts {
6793 if opt != nil {
6894 opt(t)
6995 }
7096 }
97+ go t.writeLoop()
7198 return t
7299 }
73100
74-// Send writes f as a single frame: a 4-byte big-endian length (covering
75-// the type byte and payload), the type byte, then the payload — assembled
76-// into one buffer and written with a single Write call.
101+type sendRequest struct {
102+ data []byte
103+ done chan error // nil for SendAsync
104+ end func(bool)
105+}
106+
107+// Send queues f for the sole writer and waits for its wire attempt.
77108 func (t *unixTransport) Send(f ports.Frame) error {
109+ data, err := marshalFrame(f)
110+ if err != nil {
111+ return err
112+ }
78113 end := t.beginOperation(ports.RuntimeAdapterSendStart, uint64(len(f.Payload)))
79- n := 1 + len(f.Payload) // type + payload
80- if n > maxFrameLen {
114+ result := make(chan error, 1)
115+ if err := t.enqueue(sendRequest{data: data, done: result, end: end}); err != nil {
81116 end(false)
82- return ErrFrameTooLarge
117+ return err
83118 }
119+ select {
120+ case err := <-result:
121+ return err
122+ case <-t.done:
123+ return errClosed
124+ }
125+}
126+
127+// SendAsync accepts f for ordered background transmission without waiting for
128+// a socket write. The queue is bounded and ownership includes a payload copy.
129+func (t *unixTransport) SendAsync(f ports.Frame) error {
130+ data, err := marshalFrame(f)
131+ if err != nil {
132+ return err
133+ }
134+ end := t.beginOperation(ports.RuntimeAdapterSendStart, uint64(len(f.Payload)))
135+ if err := t.enqueue(sendRequest{data: data, end: end}); err != nil {
136+ end(false)
137+ return err
138+ }
139+ return nil
140+}
84141
142+func marshalFrame(f ports.Frame) ([]byte, error) {
143+ n := 1 + len(f.Payload) // type + payload
144+ if n > maxFrameLen {
145+ return nil, ErrFrameTooLarge
146+ }
85147 buf := make([]byte, frameHeaderLen+n)
86148 binary.BigEndian.PutUint32(buf[:frameHeaderLen], uint32(n))
87149 buf[frameHeaderLen] = byte(f.Type)
88150 copy(buf[frameHeaderLen+1:], f.Payload)
151+ return buf, nil
152+}
89153
90- _, err := t.conn.Write(buf)
91- end(err == nil)
92- return err
154+func (t *unixTransport) enqueue(req sendRequest) error {
155+ t.egressMu.Lock()
156+ defer t.egressMu.Unlock()
157+ if t.egressClosing {
158+ return errClosed
159+ }
160+ select {
161+ case t.egress <- req:
162+ return nil
163+ default:
164+ return ErrBackpressure
165+ }
166+}
167+
168+func (t *unixTransport) writeLoop() {
169+ defer close(t.writerDone)
170+ for {
171+ select {
172+ case req := <-t.egress:
173+ t.completeWrite(req, writeAll(t.conn, req.data))
174+ case <-t.done:
175+ t.egressMu.Lock()
176+ for {
177+ select {
178+ case req := <-t.egress:
179+ t.completeWrite(req, errClosed)
180+ default:
181+ t.egressMu.Unlock()
182+ return
183+ }
184+ }
185+ }
186+ }
187+}
188+
189+func (t *unixTransport) completeWrite(req sendRequest, err error) {
190+ if req.end != nil {
191+ req.end(err == nil)
192+ }
193+ if req.done != nil {
194+ req.done <- err
195+ }
196+}
197+
198+func writeAll(w io.Writer, data []byte) error {
199+ for len(data) > 0 {
200+ n, err := w.Write(data)
201+ if n > 0 {
202+ data = data[n:]
203+ }
204+ if err != nil {
205+ return err
206+ }
207+ if n == 0 {
208+ return io.ErrShortWrite
209+ }
210+ }
211+ return nil
93212 }
94213
95214 // Recv reads one frame: a 4-byte length, then a body of that many bytes
@@ -154,7 +273,7 @@ func (t *unixTransport) beginOperation(start ports.RuntimeMarkKind, bytes uint64
154273 func (t *unixTransport) beginObservedOperation() bool {
155274 t.operationMu.Lock()
156275 defer t.operationMu.Unlock()
157- if t.closing {
276+ if t.observedClosing {
158277 return false
159278 }
160279 if t.operationCount == 0 {
@@ -176,20 +295,27 @@ func (t *unixTransport) finishObservedOperation() {
176295 func (t *unixTransport) beginShutdown() <-chan struct{} {
177296 t.operationMu.Lock()
178297 defer t.operationMu.Unlock()
179- t.closing = true
298+ t.observedClosing = true
180299 if t.operationCount == 0 {
181300 return nil
182301 }
183302 return t.operationsDone
184303 }
185304
186-// Close closes the underlying connection and waits for any interrupted
187-// carriage operation to write its matching failed end mark.
305+// Close interrupts Recv and the writer's current Write, fails queued frames,
306+// and waits until the sole writer and all observed operations have stopped.
188307 func (t *unixTransport) Close() error {
189- done := t.beginShutdown()
190- err := t.conn.Close()
191- if done != nil {
192- <-done
193- }
194- return err
308+ t.closeOnce.Do(func() {
309+ operationsDone := t.beginShutdown()
310+ t.egressMu.Lock()
311+ t.egressClosing = true
312+ close(t.done)
313+ t.egressMu.Unlock()
314+ t.closeErr = t.conn.Close()
315+ <-t.writerDone
316+ if operationsDone != nil {
317+ <-operationsDone
318+ }
319+ })
320+ return t.closeErr
195321 }
@@ -27,15 +27,25 @@ var ErrZeroLengthFrame = errors.New("ipc: zero-length frame")
27 // maxFrameLen, and by Send when a payload is too large to encode.27 // maxFrameLen, and by Send when a payload is too large to encode.
28 var ErrFrameTooLarge = errors.New("ipc: frame exceeds maximum length")28 var ErrFrameTooLarge = errors.New("ipc: frame exceeds maximum length")
29 29
30+// ErrBackpressure means the bounded IPC egress queue is full. Callers using
31+// SendAsync must treat it as a failed render emission rather than spawning an
32+// unbounded queue of their own.
33+var ErrBackpressure = errors.New("ipc: egress queue full")
34+
35+var errClosed = errors.New("ipc: closed")
36+
37+// sendQueueCapacity bounds frames accepted while a peer is not reading. It is
38+// deliberately small: render output can be large and must never turn a wedged
39+// peer into an unbounded memory commitment.
40+const sendQueueCapacity = 8
41+
30 // unixTransport implements ports.Transport over a net.Conn (in practice an42 // unixTransport implements ports.Transport over a net.Conn (in practice an
31 // AF_UNIX SOCK_STREAM connection, but any net.Conn works — this also lets43 // AF_UNIX SOCK_STREAM connection, but any net.Conn works — this also lets
32 // tests exercise it over net.Pipe).44 // tests exercise it over net.Pipe).
33 //45 //
34-// unixTransport is safe for one concurrent caller of Send and one46+// unixTransport has one writer goroutine which owns every conn.Write. Send
35-// concurrent caller of Recv at the same time (i.e. a single reader47+// waits for that owner's wire attempt; SendAsync only performs a bounded,
36-// goroutine and a single writer goroutine); it is not safe for concurrent48+// ordered enqueue. Recv still has one caller at a time.
37-// calls to Send from multiple goroutines, nor Recv from multiple
38-// goroutines.
39 type Option func(*unixTransport)49 type Option func(*unixTransport)
40 50
41 // WithRuntimeObserver enables process-local transport marks. It accepts no51 // WithRuntimeObserver enables process-local transport marks. It accepts no
@@ -45,12 +55,23 @@ func WithRuntimeObserver(observer ports.SerializedRuntimeObserver) Option {
45 }55 }
46 56
47 type unixTransport struct {57 type unixTransport struct {
48- conn net.Conn58+ conn net.Conn
49- observer ports.SerializedRuntimeObserver59+ observer ports.SerializedRuntimeObserver
50- operationMu sync.Mutex60+
51- operationCount int61+ operationMu sync.Mutex
52- closing bool62+ operationCount int
53- operationsDone chan struct{}63+ observedClosing bool
64+ operationsDone chan struct{}
65+
66+ // egressMu synchronizes Close with nonblocking enqueue. Once closed, no
67+ // request can be left behind after the writer drains the queue.
68+ egressMu sync.Mutex
69+ egressClosing bool
70+ egress chan sendRequest
71+ done chan struct{}
72+ writerDone chan struct{}
73+ closeOnce sync.Once
74+ closeErr error
54 75
55 // readBuf is reused across Recv calls (grow-once strategy): it grows76 // readBuf is reused across Recv calls (grow-once strategy): it grows
56 // to fit the largest frame seen so far and is never shrunk. The Frame77 // to fit the largest frame seen so far and is never shrunk. The Frame
@@ -62,34 +83,132 @@ type unixTransport struct {
62 // NewTransport wraps conn as a ports.Transport speaking vev's framed83 // NewTransport wraps conn as a ports.Transport speaking vev's framed
63 // binary protocol.84 // binary protocol.
64 func NewTransport(conn net.Conn, opts ...Option) ports.Transport {85 func NewTransport(conn net.Conn, opts ...Option) ports.Transport {
65- t := &unixTransport{conn: conn}86+ t := &unixTransport{
87+ conn: conn,
88+ egress: make(chan sendRequest, sendQueueCapacity),
89+ done: make(chan struct{}),
90+ writerDone: make(chan struct{}),
91+ }
66 for _, opt := range opts {92 for _, opt := range opts {
67 if opt != nil {93 if opt != nil {
68 opt(t)94 opt(t)
69 }95 }
70 }96 }
97+ go t.writeLoop()
71 return t98 return t
72 }99 }
73 100
74-// Send writes f as a single frame: a 4-byte big-endian length (covering101+type sendRequest struct {
75-// the type byte and payload), the type byte, then the payload — assembled102+ data []byte
76-// into one buffer and written with a single Write call.103+ done chan error // nil for SendAsync
104+ end func(bool)
105+}
106+
107+// Send queues f for the sole writer and waits for its wire attempt.
77 func (t *unixTransport) Send(f ports.Frame) error {108 func (t *unixTransport) Send(f ports.Frame) error {
109+ data, err := marshalFrame(f)
110+ if err != nil {
111+ return err
112+ }
78 end := t.beginOperation(ports.RuntimeAdapterSendStart, uint64(len(f.Payload)))113 end := t.beginOperation(ports.RuntimeAdapterSendStart, uint64(len(f.Payload)))
79- n := 1 + len(f.Payload) // type + payload114+ result := make(chan error, 1)
80- if n > maxFrameLen {115+ if err := t.enqueue(sendRequest{data: data, done: result, end: end}); err != nil {
81 end(false)116 end(false)
82- return ErrFrameTooLarge117+ return err
83 }118 }
119+ select {
120+ case err := <-result:
121+ return err
122+ case <-t.done:
123+ return errClosed
124+ }
125+}
126+
127+// SendAsync accepts f for ordered background transmission without waiting for
128+// a socket write. The queue is bounded and ownership includes a payload copy.
129+func (t *unixTransport) SendAsync(f ports.Frame) error {
130+ data, err := marshalFrame(f)
131+ if err != nil {
132+ return err
133+ }
134+ end := t.beginOperation(ports.RuntimeAdapterSendStart, uint64(len(f.Payload)))
135+ if err := t.enqueue(sendRequest{data: data, end: end}); err != nil {
136+ end(false)
137+ return err
138+ }
139+ return nil
140+}
84 141
142+func marshalFrame(f ports.Frame) ([]byte, error) {
143+ n := 1 + len(f.Payload) // type + payload
144+ if n > maxFrameLen {
145+ return nil, ErrFrameTooLarge
146+ }
85 buf := make([]byte, frameHeaderLen+n)147 buf := make([]byte, frameHeaderLen+n)
86 binary.BigEndian.PutUint32(buf[:frameHeaderLen], uint32(n))148 binary.BigEndian.PutUint32(buf[:frameHeaderLen], uint32(n))
87 buf[frameHeaderLen] = byte(f.Type)149 buf[frameHeaderLen] = byte(f.Type)
88 copy(buf[frameHeaderLen+1:], f.Payload)150 copy(buf[frameHeaderLen+1:], f.Payload)
151+ return buf, nil
152+}
89 153
90- _, err := t.conn.Write(buf)154+func (t *unixTransport) enqueue(req sendRequest) error {
91- end(err == nil)155+ t.egressMu.Lock()
92- return err156+ defer t.egressMu.Unlock()
157+ if t.egressClosing {
158+ return errClosed
159+ }
160+ select {
161+ case t.egress <- req:
162+ return nil
163+ default:
164+ return ErrBackpressure
165+ }
166+}
167+
168+func (t *unixTransport) writeLoop() {
169+ defer close(t.writerDone)
170+ for {
171+ select {
172+ case req := <-t.egress:
173+ t.completeWrite(req, writeAll(t.conn, req.data))
174+ case <-t.done:
175+ t.egressMu.Lock()
176+ for {
177+ select {
178+ case req := <-t.egress:
179+ t.completeWrite(req, errClosed)
180+ default:
181+ t.egressMu.Unlock()
182+ return
183+ }
184+ }
185+ }
186+ }
187+}
188+
189+func (t *unixTransport) completeWrite(req sendRequest, err error) {
190+ if req.end != nil {
191+ req.end(err == nil)
192+ }
193+ if req.done != nil {
194+ req.done <- err
195+ }
196+}
197+
198+func writeAll(w io.Writer, data []byte) error {
199+ for len(data) > 0 {
200+ n, err := w.Write(data)
201+ if n > 0 {
202+ data = data[n:]
203+ }
204+ if err != nil {
205+ return err
206+ }
207+ if n == 0 {
208+ return io.ErrShortWrite
209+ }
210+ }
211+ return nil
93 }212 }
94 213
95 // Recv reads one frame: a 4-byte length, then a body of that many bytes214 // Recv reads one frame: a 4-byte length, then a body of that many bytes
@@ -154,7 +273,7 @@ func (t *unixTransport) beginOperation(start ports.RuntimeMarkKind, bytes uint64
154 func (t *unixTransport) beginObservedOperation() bool {273 func (t *unixTransport) beginObservedOperation() bool {
155 t.operationMu.Lock()274 t.operationMu.Lock()
156 defer t.operationMu.Unlock()275 defer t.operationMu.Unlock()
157- if t.closing {276+ if t.observedClosing {
158 return false277 return false
159 }278 }
160 if t.operationCount == 0 {279 if t.operationCount == 0 {
@@ -176,20 +295,27 @@ func (t *unixTransport) finishObservedOperation() {
176 func (t *unixTransport) beginShutdown() <-chan struct{} {295 func (t *unixTransport) beginShutdown() <-chan struct{} {
177 t.operationMu.Lock()296 t.operationMu.Lock()
178 defer t.operationMu.Unlock()297 defer t.operationMu.Unlock()
179- t.closing = true298+ t.observedClosing = true
180 if t.operationCount == 0 {299 if t.operationCount == 0 {
181 return nil300 return nil
182 }301 }
183 return t.operationsDone302 return t.operationsDone
184 }303 }
185 304
186-// Close closes the underlying connection and waits for any interrupted305+// Close interrupts Recv and the writer's current Write, fails queued frames,
187-// carriage operation to write its matching failed end mark.306+// and waits until the sole writer and all observed operations have stopped.
188 func (t *unixTransport) Close() error {307 func (t *unixTransport) Close() error {
189- done := t.beginShutdown()308+ t.closeOnce.Do(func() {
190- err := t.conn.Close()309+ operationsDone := t.beginShutdown()
191- if done != nil {310+ t.egressMu.Lock()
192- <-done311+ t.egressClosing = true
193- }312+ close(t.done)
194- return err313+ t.egressMu.Unlock()
314+ t.closeErr = t.conn.Close()
315+ <-t.writerDone
316+ if operationsDone != nil {
317+ <-operationsDone
318+ }
319+ })
320+ return t.closeErr
195 }321 }
M internal/adapters/ipc/transport_test.go +101 -0
@@ -8,6 +8,7 @@ import (
88 "reflect"
99 "sync"
1010 "testing"
11+ "time"
1112
1213 "github.com/bnema/vev/internal/domain"
1314 "github.com/bnema/vev/internal/ports"
@@ -231,3 +232,103 @@ func TestTransportSendOversizePayloadRejected(t *testing.T) {
231232 t.Fatalf("client.Send() error = %v, want ErrFrameTooLarge", err)
232233 }
233234 }
235+
236+func TestTransportAsyncEgressPreservesWelcomeBeforeOutput(t *testing.T) {
237+ serverConn, clientConn := net.Pipe()
238+ defer func() { _ = serverConn.Close() }()
239+ defer func() { _ = clientConn.Close() }()
240+
241+ server := NewTransport(serverConn)
242+ client := NewTransport(clientConn)
243+ async, ok := server.(ports.AsyncTransport)
244+ if !ok {
245+ t.Fatal("IPC transport does not implement AsyncTransport")
246+ }
247+
248+ welcome := ports.Frame{Type: ports.MsgWelcome, Payload: []byte("welcome")}
249+ sent := make(chan error, 1)
250+ go func() { sent <- server.Send(welcome) }()
251+ got, err := client.Recv()
252+ if err != nil {
253+ t.Fatalf("Recv welcome: %v", err)
254+ }
255+ if !reflect.DeepEqual(got, welcome) {
256+ t.Fatalf("welcome = %#v, want %#v", got, welcome)
257+ }
258+ if err := <-sent; err != nil {
259+ t.Fatalf("Send welcome: %v", err)
260+ }
261+
262+ payload := []byte("first")
263+ if err := async.SendAsync(ports.Frame{Type: ports.MsgOutput, Payload: payload}); err != nil {
264+ t.Fatalf("SendAsync first output: %v", err)
265+ }
266+ payload[0] = 'X' // async ownership must not retain caller memory.
267+ if err := async.SendAsync(ports.Frame{Type: ports.MsgOutput, Payload: []byte("second")}); err != nil {
268+ t.Fatalf("SendAsync second output: %v", err)
269+ }
270+ for _, want := range []ports.Frame{
271+ {Type: ports.MsgOutput, Payload: []byte("first")},
272+ {Type: ports.MsgOutput, Payload: []byte("second")},
273+ } {
274+ got, err := client.Recv()
275+ if err != nil {
276+ t.Fatalf("Recv output: %v", err)
277+ }
278+ if !reflect.DeepEqual(got, want) {
279+ t.Fatalf("output = %#v, want %#v", got, want)
280+ }
281+ }
282+}
283+
284+func TestTransportAsyncEgressIsBoundedAndCloseInterruptsWorkers(t *testing.T) {
285+ serverConn, clientConn := net.Pipe()
286+ defer func() { _ = clientConn.Close() }()
287+
288+ tr := NewTransport(serverConn)
289+ async, ok := tr.(ports.AsyncTransport)
290+ if !ok {
291+ t.Fatal("IPC transport does not implement AsyncTransport")
292+ }
293+
294+ // The first write blocks in net.Pipe because the peer never drains it.
295+ if err := async.SendAsync(ports.Frame{Type: ports.MsgOutput, Payload: []byte("blocked")}); err != nil {
296+ t.Fatalf("SendAsync blocked frame: %v", err)
297+ }
298+ var backpressure error
299+ for range sendQueueCapacity + 2 {
300+ err := async.SendAsync(ports.Frame{Type: ports.MsgOutput, Payload: []byte("queued")})
301+ if errors.Is(err, ErrBackpressure) {
302+ backpressure = err
303+ break
304+ }
305+ if err != nil {
306+ t.Fatalf("SendAsync unexpected error: %v", err)
307+ }
308+ }
309+ if !errors.Is(backpressure, ErrBackpressure) {
310+ t.Fatalf("SendAsync never reported bounded backpressure, got %v", backpressure)
311+ }
312+
313+ recvDone := make(chan error, 1)
314+ go func() {
315+ _, err := tr.Recv()
316+ recvDone <- err
317+ }()
318+ if err := tr.Close(); err != nil {
319+ t.Fatalf("Close: %v", err)
320+ }
321+ select {
322+ case err := <-recvDone:
323+ if err == nil {
324+ t.Fatal("Recv succeeded after Close")
325+ }
326+ case <-time.After(time.Second):
327+ t.Fatal("Close did not interrupt Recv")
328+ }
329+ select {
330+ case <-tr.(*unixTransport).writerDone:
331+ default:
332+ t.Fatal("Close returned before the IPC writer terminated")
333+ }
334+}
@@ -8,6 +8,7 @@ import (
8 "reflect"8 "reflect"
9 "sync"9 "sync"
10 "testing"10 "testing"
11+ "time"
11 12
12 "github.com/bnema/vev/internal/domain"13 "github.com/bnema/vev/internal/domain"
13 "github.com/bnema/vev/internal/ports"14 "github.com/bnema/vev/internal/ports"
@@ -231,3 +232,103 @@ func TestTransportSendOversizePayloadRejected(t *testing.T) {
231 t.Fatalf("client.Send() error = %v, want ErrFrameTooLarge", err)232 t.Fatalf("client.Send() error = %v, want ErrFrameTooLarge", err)
232 }233 }
233 }234 }
235+
236+func TestTransportAsyncEgressPreservesWelcomeBeforeOutput(t *testing.T) {
237+ serverConn, clientConn := net.Pipe()
238+ defer func() { _ = serverConn.Close() }()
239+ defer func() { _ = clientConn.Close() }()
240+
241+ server := NewTransport(serverConn)
242+ client := NewTransport(clientConn)
243+ async, ok := server.(ports.AsyncTransport)
244+ if !ok {
245+ t.Fatal("IPC transport does not implement AsyncTransport")
246+ }
247+
248+ welcome := ports.Frame{Type: ports.MsgWelcome, Payload: []byte("welcome")}
249+ sent := make(chan error, 1)
250+ go func() { sent <- server.Send(welcome) }()
251+ got, err := client.Recv()
252+ if err != nil {
253+ t.Fatalf("Recv welcome: %v", err)
254+ }
255+ if !reflect.DeepEqual(got, welcome) {
256+ t.Fatalf("welcome = %#v, want %#v", got, welcome)
257+ }
258+ if err := <-sent; err != nil {
259+ t.Fatalf("Send welcome: %v", err)
260+ }
261+
262+ payload := []byte("first")
263+ if err := async.SendAsync(ports.Frame{Type: ports.MsgOutput, Payload: payload}); err != nil {
264+ t.Fatalf("SendAsync first output: %v", err)
265+ }
266+ payload[0] = 'X' // async ownership must not retain caller memory.
267+ if err := async.SendAsync(ports.Frame{Type: ports.MsgOutput, Payload: []byte("second")}); err != nil {
268+ t.Fatalf("SendAsync second output: %v", err)
269+ }
270+ for _, want := range []ports.Frame{
271+ {Type: ports.MsgOutput, Payload: []byte("first")},
272+ {Type: ports.MsgOutput, Payload: []byte("second")},
273+ } {
274+ got, err := client.Recv()
275+ if err != nil {
276+ t.Fatalf("Recv output: %v", err)
277+ }
278+ if !reflect.DeepEqual(got, want) {
279+ t.Fatalf("output = %#v, want %#v", got, want)
280+ }
281+ }
282+}
283+
284+func TestTransportAsyncEgressIsBoundedAndCloseInterruptsWorkers(t *testing.T) {
285+ serverConn, clientConn := net.Pipe()
286+ defer func() { _ = clientConn.Close() }()
287+
288+ tr := NewTransport(serverConn)
289+ async, ok := tr.(ports.AsyncTransport)
290+ if !ok {
291+ t.Fatal("IPC transport does not implement AsyncTransport")
292+ }
293+
294+ // The first write blocks in net.Pipe because the peer never drains it.
295+ if err := async.SendAsync(ports.Frame{Type: ports.MsgOutput, Payload: []byte("blocked")}); err != nil {
296+ t.Fatalf("SendAsync blocked frame: %v", err)
297+ }
298+ var backpressure error
299+ for range sendQueueCapacity + 2 {
300+ err := async.SendAsync(ports.Frame{Type: ports.MsgOutput, Payload: []byte("queued")})
301+ if errors.Is(err, ErrBackpressure) {
302+ backpressure = err
303+ break
304+ }
305+ if err != nil {
306+ t.Fatalf("SendAsync unexpected error: %v", err)
307+ }
308+ }
309+ if !errors.Is(backpressure, ErrBackpressure) {
310+ t.Fatalf("SendAsync never reported bounded backpressure, got %v", backpressure)
311+ }
312+
313+ recvDone := make(chan error, 1)
314+ go func() {
315+ _, err := tr.Recv()
316+ recvDone <- err
317+ }()
318+ if err := tr.Close(); err != nil {
319+ t.Fatalf("Close: %v", err)
320+ }
321+ select {
322+ case err := <-recvDone:
323+ if err == nil {
324+ t.Fatal("Recv succeeded after Close")
325+ }
326+ case <-time.After(time.Second):
327+ t.Fatal("Close did not interrupt Recv")
328+ }
329+ select {
330+ case <-tr.(*unixTransport).writerDone:
331+ default:
332+ t.Fatal("Close returned before the IPC writer terminated")
333+ }
334+}
M internal/adapters/pty/pty_linux.go +24 -4
@@ -97,6 +97,20 @@ func (Factory) Open(ctx context.Context, command string, args []string, env []st
9797 Setctty: true,
9898 Ctty: 0,
9999 }
100+ // CommandContext's default cancellation kills only the direct child. The
101+ // child leads a process group, so cancellation must target that group too.
102+ cmd.Cancel = func() error {
103+ if cmd.Process == nil {
104+ return os.ErrProcessDone
105+ }
106+ if err := signalProcessGroup(cmd.Process.Pid, syscall.SIGKILL); err != nil {
107+ if errors.Is(err, syscall.ESRCH) {
108+ return os.ErrProcessDone
109+ }
110+ return err
111+ }
112+ return nil
113+ }
100114
101115 if err := cmd.Start(); err != nil {
102116 return nil, fmt.Errorf("pty: start %q: %w", command, err)
@@ -194,9 +208,8 @@ func (p *linuxPTY) Close() error {
194208 p.closeOnce.Do(func() {
195209 pid := p.cmd.Process.Pid
196210
197- // Negative pid targets the whole process group (the child is the group
198- // leader thanks to Setsid). Ignore errors: the child may already be gone.
199- _ = unix.Kill(-pid, unix.SIGHUP)
211+ // Ignore errors: the child may already be gone.
212+ _ = signalProcessGroup(pid, syscall.SIGHUP)
200213
201214 // Reap the child (exit status is intentionally discarded; the port has no
202215 // Wait — "child gone" surfaces to callers as io.EOF on Read).
@@ -209,7 +222,7 @@ func (p *linuxPTY) Close() error {
209222 select {
210223 case <-reaped:
211224 case <-time.After(killGracePeriod):
212- _ = unix.Kill(-pid, unix.SIGKILL)
225+ _ = signalProcessGroup(pid, syscall.SIGKILL)
213226 <-reaped
214227 }
215228
@@ -218,6 +231,13 @@ func (p *linuxPTY) Close() error {
218231 return p.closeErr
219232 }
220233
234+// signalProcessGroup signals the process group led by pid. Open creates each
235+// child as its own session and process-group leader, so a negative pid reaches
236+// both the command and every descendant that remains in its process group.
237+func signalProcessGroup(pid int, signal syscall.Signal) error {
238+ return unix.Kill(-pid, signal)
239+}
240+
221241 // setWinsize applies sz to the terminal referenced by fd via TIOCSWINSZ.
222242 func setWinsize(fd int, sz domain.Size) error {
223243 return unix.IoctlSetWinsize(fd, unix.TIOCSWINSZ, &unix.Winsize{
@@ -97,6 +97,20 @@ func (Factory) Open(ctx context.Context, command string, args []string, env []st
97 Setctty: true,97 Setctty: true,
98 Ctty: 0,98 Ctty: 0,
99 }99 }
100+ // CommandContext's default cancellation kills only the direct child. The
101+ // child leads a process group, so cancellation must target that group too.
102+ cmd.Cancel = func() error {
103+ if cmd.Process == nil {
104+ return os.ErrProcessDone
105+ }
106+ if err := signalProcessGroup(cmd.Process.Pid, syscall.SIGKILL); err != nil {
107+ if errors.Is(err, syscall.ESRCH) {
108+ return os.ErrProcessDone
109+ }
110+ return err
111+ }
112+ return nil
113+ }
100 114
101 if err := cmd.Start(); err != nil {115 if err := cmd.Start(); err != nil {
102 return nil, fmt.Errorf("pty: start %q: %w", command, err)116 return nil, fmt.Errorf("pty: start %q: %w", command, err)
@@ -194,9 +208,8 @@ func (p *linuxPTY) Close() error {
194 p.closeOnce.Do(func() {208 p.closeOnce.Do(func() {
195 pid := p.cmd.Process.Pid209 pid := p.cmd.Process.Pid
196 210
197- // Negative pid targets the whole process group (the child is the group211+ // Ignore errors: the child may already be gone.
198- // leader thanks to Setsid). Ignore errors: the child may already be gone.212+ _ = signalProcessGroup(pid, syscall.SIGHUP)
199- _ = unix.Kill(-pid, unix.SIGHUP)
200 213
201 // Reap the child (exit status is intentionally discarded; the port has no214 // Reap the child (exit status is intentionally discarded; the port has no
202 // Wait — "child gone" surfaces to callers as io.EOF on Read).215 // Wait — "child gone" surfaces to callers as io.EOF on Read).
@@ -209,7 +222,7 @@ func (p *linuxPTY) Close() error {
209 select {222 select {
210 case <-reaped:223 case <-reaped:
211 case <-time.After(killGracePeriod):224 case <-time.After(killGracePeriod):
212- _ = unix.Kill(-pid, unix.SIGKILL)225+ _ = signalProcessGroup(pid, syscall.SIGKILL)
213 <-reaped226 <-reaped
214 }227 }
215 228
@@ -218,6 +231,13 @@ func (p *linuxPTY) Close() error {
218 return p.closeErr231 return p.closeErr
219 }232 }
220 233
234+// signalProcessGroup signals the process group led by pid. Open creates each
235+// child as its own session and process-group leader, so a negative pid reaches
236+// both the command and every descendant that remains in its process group.
237+func signalProcessGroup(pid int, signal syscall.Signal) error {
238+ return unix.Kill(-pid, signal)
239+}
240+
221 // setWinsize applies sz to the terminal referenced by fd via TIOCSWINSZ.241 // setWinsize applies sz to the terminal referenced by fd via TIOCSWINSZ.
222 func setWinsize(fd int, sz domain.Size) error {242 func setWinsize(fd int, sz domain.Size) error {
223 return unix.IoctlSetWinsize(fd, unix.TIOCSWINSZ, &unix.Winsize{243 return unix.IoctlSetWinsize(fd, unix.TIOCSWINSZ, &unix.Winsize{
M internal/adapters/pty/pty_linux_test.go +36 -0
@@ -205,6 +205,42 @@ func TestClose_TerminatesLongRunningChild(t *testing.T) {
205205 }, 3*time.Second, 20*time.Millisecond)
206206 }
207207
208+func TestOpen_ContextCancellationTerminatesProcessGroup(t *testing.T) {
209+ if testing.Short() {
210+ t.Skip("skipping pty integration test in -short mode")
211+ }
212+ ctx, cancel := context.WithCancel(context.Background())
213+ defer cancel()
214+
215+ // The background sleep ignores SIGHUP so killing only the session leader
216+ // cannot accidentally pass via the terminal's hangup behavior.
217+ p, err := newFactory().Open(ctx, "sh", []string{"-c", `(trap '' HUP; exec sleep 60) & child=$!; printf '%s\n' "$child"; wait "$child"`}, os.Environ(), "", domain.Size{Cols: 80, Rows: 24})
218+ require.NoError(t, err)
219+ t.Cleanup(func() { _ = p.Close() })
220+
221+ buf := make([]byte, 64)
222+ n, err := p.Read(buf)
223+ require.NoError(t, err)
224+ descendantPID, err := strconv.Atoi(strings.TrimSpace(string(buf[:n])))
225+ require.NoError(t, err)
226+
227+ cancel()
228+
229+ // The direct shell may remain as a zombie until Close reaps it; a zombie has
230+ // exited, so it and the background sleep must not remain running.
231+ require.Eventually(t, func() bool {
232+ return processTerminated(p.Pid()) && processTerminated(descendantPID)
233+ }, 3*time.Second, 20*time.Millisecond)
234+}
235+
236+func processTerminated(pid int) bool {
237+ if errors.Is(syscall.Kill(pid, 0), syscall.ESRCH) {
238+ return true
239+ }
240+ status, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/status")
241+ return err != nil || strings.Contains(string(status), "State:\tZ")
242+}
243+
208244 func TestResize_DeliversSIGWINCH(t *testing.T) {
209245 if testing.Short() {
210246 t.Skip("skipping pty integration test in -short mode")
@@ -205,6 +205,42 @@ func TestClose_TerminatesLongRunningChild(t *testing.T) {
205 }, 3*time.Second, 20*time.Millisecond)205 }, 3*time.Second, 20*time.Millisecond)
206 }206 }
207 207
208+func TestOpen_ContextCancellationTerminatesProcessGroup(t *testing.T) {
209+ if testing.Short() {
210+ t.Skip("skipping pty integration test in -short mode")
211+ }
212+ ctx, cancel := context.WithCancel(context.Background())
213+ defer cancel()
214+
215+ // The background sleep ignores SIGHUP so killing only the session leader
216+ // cannot accidentally pass via the terminal's hangup behavior.
217+ p, err := newFactory().Open(ctx, "sh", []string{"-c", `(trap '' HUP; exec sleep 60) & child=$!; printf '%s\n' "$child"; wait "$child"`}, os.Environ(), "", domain.Size{Cols: 80, Rows: 24})
218+ require.NoError(t, err)
219+ t.Cleanup(func() { _ = p.Close() })
220+
221+ buf := make([]byte, 64)
222+ n, err := p.Read(buf)
223+ require.NoError(t, err)
224+ descendantPID, err := strconv.Atoi(strings.TrimSpace(string(buf[:n])))
225+ require.NoError(t, err)
226+
227+ cancel()
228+
229+ // The direct shell may remain as a zombie until Close reaps it; a zombie has
230+ // exited, so it and the background sleep must not remain running.
231+ require.Eventually(t, func() bool {
232+ return processTerminated(p.Pid()) && processTerminated(descendantPID)
233+ }, 3*time.Second, 20*time.Millisecond)
234+}
235+
236+func processTerminated(pid int) bool {
237+ if errors.Is(syscall.Kill(pid, 0), syscall.ESRCH) {
238+ return true
239+ }
240+ status, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/status")
241+ return err != nil || strings.Contains(string(status), "State:\tZ")
242+}
243+
208 func TestResize_DeliversSIGWINCH(t *testing.T) {244 func TestResize_DeliversSIGWINCH(t *testing.T) {
209 if testing.Short() {245 if testing.Short() {
210 t.Skip("skipping pty integration test in -short mode")246 t.Skip("skipping pty integration test in -short mode")
M internal/adapters/sshstdio/transport.go +3 -3
@@ -44,7 +44,7 @@ func WithRuntimeObserver(observer ports.SerializedRuntimeObserver) Option {
4444
4545 // NewTransport wraps separate reader/writer streams as a framed Transport.
4646 func NewTransport(r io.Reader, w io.Writer, closeFn closeFunc, opts ...Option) ports.Transport {
47- t := newTransport(r, w, closeFn, nil).(*transport)
47+ t := newTransport(r, w, closeFn, nil)
4848 for _, opt := range opts {
4949 if opt != nil {
5050 opt(t)
@@ -53,7 +53,7 @@ func NewTransport(r io.Reader, w io.Writer, closeFn closeFunc, opts ...Option) p
5353 return t
5454 }
5555
56-func newTransport(r io.Reader, w io.Writer, closeFn closeFunc, eofErr eofErrFunc) ports.Transport {
56+func newTransport(r io.Reader, w io.Writer, closeFn closeFunc, eofErr eofErrFunc) *transport {
5757 if closeFn == nil {
5858 closeFn = func() error { return nil }
5959 }
@@ -361,7 +361,7 @@ func dialContext(ctx context.Context, target, session string, log *slog.Logger,
361361 }
362362
363363 waiter := newProcessWaiter(cmd, stdin, &stderr, sshCloseTimeout, log, target, session)
364- transport := newTransport(stdout, stdin, waiter.close, waiter.eofErr).(*transport)
364+ transport := newTransport(stdout, stdin, waiter.close, waiter.eofErr)
365365 for _, opt := range opts {
366366 if opt != nil {
367367 opt(transport)
@@ -44,7 +44,7 @@ func WithRuntimeObserver(observer ports.SerializedRuntimeObserver) Option {
44 44
45 // NewTransport wraps separate reader/writer streams as a framed Transport.45 // NewTransport wraps separate reader/writer streams as a framed Transport.
46 func NewTransport(r io.Reader, w io.Writer, closeFn closeFunc, opts ...Option) ports.Transport {46 func NewTransport(r io.Reader, w io.Writer, closeFn closeFunc, opts ...Option) ports.Transport {
47- t := newTransport(r, w, closeFn, nil).(*transport)47+ t := newTransport(r, w, closeFn, nil)
48 for _, opt := range opts {48 for _, opt := range opts {
49 if opt != nil {49 if opt != nil {
50 opt(t)50 opt(t)
@@ -53,7 +53,7 @@ func NewTransport(r io.Reader, w io.Writer, closeFn closeFunc, opts ...Option) p
53 return t53 return t
54 }54 }
55 55
56-func newTransport(r io.Reader, w io.Writer, closeFn closeFunc, eofErr eofErrFunc) ports.Transport {56+func newTransport(r io.Reader, w io.Writer, closeFn closeFunc, eofErr eofErrFunc) *transport {
57 if closeFn == nil {57 if closeFn == nil {
58 closeFn = func() error { return nil }58 closeFn = func() error { return nil }
59 }59 }
@@ -361,7 +361,7 @@ func dialContext(ctx context.Context, target, session string, log *slog.Logger,
361 }361 }
362 362
363 waiter := newProcessWaiter(cmd, stdin, &stderr, sshCloseTimeout, log, target, session)363 waiter := newProcessWaiter(cmd, stdin, &stderr, sshCloseTimeout, log, target, session)
364- transport := newTransport(stdout, stdin, waiter.close, waiter.eofErr).(*transport)364+ transport := newTransport(stdout, stdin, waiter.close, waiter.eofErr)
365 for _, opt := range opts {365 for _, opt := range opts {
366 if opt != nil {366 if opt != nil {
367 opt(transport)367 opt(transport)
M internal/app/run.go +11 -4
@@ -243,8 +243,15 @@ func dispatch(ctx context.Context, cmd command) error {
243243 // performanceTrace creates one serialized timestamp owner for this process.
244244 // An empty trace environment leaves all production behavior and wire bytes
245245 // unchanged.
246-// newPerformanceTrace is a test seam around composition-root trace setup.
247-var newPerformanceTrace = performanceTrace
246+// Composition-root factory seams keep observer propagation testable without
247+// opening real transports.
248+var (
249+ newPerformanceTrace = performanceTrace
250+ newRemoteDialerFactoryWithRuntimeObserver = func(observer ports.SerializedRuntimeObserver) ports.RemoteDialerFactory {
251+ return remoteadapter.NewDialerFactoryWithRuntimeObserver(observer)
252+ }
253+ runClientWithRuntimeObserver = client.RunWithRuntimeObserver
254+)
248255
249256 func performanceTrace(clk ports.Clock) (ports.SerializedRuntimeObserver, io.Closer, error) {
250257 return performanceTraceWithFactories(clk, observability.NewJSONL, ports.NewRuntimeCorrelationObserver)
@@ -475,10 +482,10 @@ func runAttach(ctx context.Context, intent uint8, name, remoteTarget string) (re
475482 localDialer: func() ports.Dialer {
476483 return localDaemonDialer{dir: ipc.SocketDir(), observer: observer}
477484 },
478- remoteDialerFactory: remoteadapter.NewDialerFactoryWithRuntimeObserver(observer),
485+ remoteDialerFactory: newRemoteDialerFactoryWithRuntimeObserver(observer),
479486 selectedRemoteTransport: os.Getenv(envRemoteTransport),
480487 runClient: func(ctx context.Context, dialer ports.Dialer, terminal ports.Terminal, behaviorClock ports.Clock, intent uint8, name string, remote bool, clipboard ports.ClipboardReader, log *slog.Logger) error {
481- return client.RunWithRuntimeObserver(ctx, dialer, terminal, behaviorClock, intent, name, remote, clipboard, log, observer)
488+ return runClientWithRuntimeObserver(ctx, dialer, terminal, behaviorClock, intent, name, remote, clipboard, log, observer)
482489 },
483490 createDetached: createDetachedLocalSession,
484491 clipboard: clipboard.New(),
@@ -243,8 +243,15 @@ func dispatch(ctx context.Context, cmd command) error {
243 // performanceTrace creates one serialized timestamp owner for this process.243 // performanceTrace creates one serialized timestamp owner for this process.
244 // An empty trace environment leaves all production behavior and wire bytes244 // An empty trace environment leaves all production behavior and wire bytes
245 // unchanged.245 // unchanged.
246-// newPerformanceTrace is a test seam around composition-root trace setup.246+// Composition-root factory seams keep observer propagation testable without
247-var newPerformanceTrace = performanceTrace247+// opening real transports.
248+var (
249+ newPerformanceTrace = performanceTrace
250+ newRemoteDialerFactoryWithRuntimeObserver = func(observer ports.SerializedRuntimeObserver) ports.RemoteDialerFactory {
251+ return remoteadapter.NewDialerFactoryWithRuntimeObserver(observer)
252+ }
253+ runClientWithRuntimeObserver = client.RunWithRuntimeObserver
254+)
248 255
249 func performanceTrace(clk ports.Clock) (ports.SerializedRuntimeObserver, io.Closer, error) {256 func performanceTrace(clk ports.Clock) (ports.SerializedRuntimeObserver, io.Closer, error) {
250 return performanceTraceWithFactories(clk, observability.NewJSONL, ports.NewRuntimeCorrelationObserver)257 return performanceTraceWithFactories(clk, observability.NewJSONL, ports.NewRuntimeCorrelationObserver)
@@ -475,10 +482,10 @@ func runAttach(ctx context.Context, intent uint8, name, remoteTarget string) (re
475 localDialer: func() ports.Dialer {482 localDialer: func() ports.Dialer {
476 return localDaemonDialer{dir: ipc.SocketDir(), observer: observer}483 return localDaemonDialer{dir: ipc.SocketDir(), observer: observer}
477 },484 },
478- remoteDialerFactory: remoteadapter.NewDialerFactoryWithRuntimeObserver(observer),485+ remoteDialerFactory: newRemoteDialerFactoryWithRuntimeObserver(observer),
479 selectedRemoteTransport: os.Getenv(envRemoteTransport),486 selectedRemoteTransport: os.Getenv(envRemoteTransport),
480 runClient: func(ctx context.Context, dialer ports.Dialer, terminal ports.Terminal, behaviorClock ports.Clock, intent uint8, name string, remote bool, clipboard ports.ClipboardReader, log *slog.Logger) error {487 runClient: func(ctx context.Context, dialer ports.Dialer, terminal ports.Terminal, behaviorClock ports.Clock, intent uint8, name string, remote bool, clipboard ports.ClipboardReader, log *slog.Logger) error {
481- return client.RunWithRuntimeObserver(ctx, dialer, terminal, behaviorClock, intent, name, remote, clipboard, log, observer)488+ return runClientWithRuntimeObserver(ctx, dialer, terminal, behaviorClock, intent, name, remote, clipboard, log, observer)
482 },489 },
483 createDetached: createDetachedLocalSession,490 createDetached: createDetachedLocalSession,
484 clipboard: clipboard.New(),491 clipboard: clipboard.New(),
M internal/app/transport_observability_test.go +91 -17
@@ -6,6 +6,7 @@ import (
66 "encoding/json"
77 "errors"
88 "io"
9+ "log/slog"
910 "os"
1011 "path/filepath"
1112 "strconv"
@@ -15,6 +16,7 @@ import (
1516
1617 "github.com/bnema/vev/internal/ports"
1718 portsmocks "github.com/bnema/vev/internal/ports/mocks"
19+ "github.com/stretchr/testify/mock"
1820 )
1921
2022 func TestPerformanceTraceUsesHarnessProcessMapping(t *testing.T) {
@@ -46,7 +48,11 @@ func TestPerformanceTraceUsesHarnessProcessMapping(t *testing.T) {
4648 if err != nil {
4749 t.Fatal(err)
4850 }
49- defer f.Close()
51+ defer func() {
52+ if err := f.Close(); err != nil {
53+ t.Errorf("close trace file: %v", err)
54+ }
55+ }()
5056 var mark struct {
5157 ProcessID string `json:"process_id"`
5258 Scenario string `json:"scenario"`
@@ -63,6 +69,48 @@ func TestPerformanceTraceUsesHarnessProcessMapping(t *testing.T) {
6369 }
6470 }
6571
72+func TestPerformanceTraceWithFactoriesCreatesOneConfiguredObserver(t *testing.T) {
73+ t.Setenv("VEV_PERF_TRACE", "trace.jsonl")
74+ t.Setenv("VEV_PERF_PROCESS_ID", "client-01")
75+ t.Setenv("VEV_PERF_SCENARIO", "lossy-remote")
76+ t.Setenv("VEV_PERF_RUN", "7")
77+
78+ clock := portsmocks.NewMockClock(t)
79+ sink := portsmocks.NewMockRuntimeObserver(t)
80+ sink.EXPECT().ObserveRuntime(mock.MatchedBy(func(mark ports.RuntimeMark) bool {
81+ return mark.Component == "client" && mark.Scenario == "lossy-remote" && mark.Run == 7 &&
82+ mark.Sequence != 0 && mark.RequestID != 0 && mark.Epoch != 0 && mark.Kind == ports.RuntimeEmitEnd
83+ })).Once()
84+
85+ var sinkCalls, correlationCalls int
86+ observer, closer, err := performanceTraceWithFactories(clock,
87+ func(path string, gotClock ports.Clock, processID string) (ports.RuntimeObserver, io.Closer, error) {
88+ sinkCalls++
89+ if path != "trace.jsonl" || gotClock != clock || processID != "client-01" {
90+ t.Fatalf("trace sink config = (%q, %v, %q)", path, gotClock, processID)
91+ }
92+ return sink, errorCloser{}, nil
93+ },
94+ func(got ports.RuntimeObserver, inputs ports.RuntimeCorrelationInputs) (ports.RuntimeObserver, error) {
95+ correlationCalls++
96+ if got != sink || inputs != (ports.RuntimeCorrelationInputs{Scenario: "lossy-remote", Run: 7}) {
97+ t.Fatalf("trace correlation config = (%v, %+v)", got, inputs)
98+ }
99+ return ports.NewRuntimeCorrelationObserver(got, inputs)
100+ },
101+ )
102+ if err != nil {
103+ t.Fatal(err)
104+ }
105+ if sinkCalls != 1 || correlationCalls != 1 || observer == nil || closer == nil {
106+ t.Fatalf("trace setup = sink calls %d, correlation calls %d, observer %v, closer %v; want one configured observer", sinkCalls, correlationCalls, observer, closer)
107+ }
108+ observer.ObserveRuntime(ports.NewRuntimeMark("client", ports.RuntimeEmitEnd, 0, true))
109+ if err := closer.Close(); err != nil {
110+ t.Fatal(err)
111+ }
112+}
113+
66114 func TestPerformanceTraceSetupRollbackJoinsCloseError(t *testing.T) {
67115 setupErr := errors.New("correlation setup failed")
68116 closeErr := errors.New("rollback close failed")
@@ -154,23 +202,49 @@ type runtimeObserverFunc func(ports.RuntimeMark)
154202
155203 func (f runtimeObserverFunc) ObserveRuntime(mark ports.RuntimeMark) { f(mark) }
156204
157-func TestPerformanceTraceAppWiresOneObserverPerProcess(t *testing.T) {
158- source, err := os.ReadFile("run.go")
159- if err != nil {
160- t.Fatalf("ReadFile(run.go): %v", err)
161- }
162- text := string(source)
163- for _, required := range []string{
164- "VEV_PERF_TRACE",
165- "VEV_PERF_PROCESS_ID",
166- "observability.NewJSONL",
167- "WithRuntimeObserver",
168- } {
169- if !strings.Contains(text, required) {
170- t.Errorf("runtime trace wiring missing %q", required)
205+func TestRunAttachPropagatesOneObserverToRemoteTransportFactory(t *testing.T) {
206+ t.Setenv("XDG_STATE_HOME", t.TempDir())
207+ t.Setenv("XDG_RUNTIME_DIR", t.TempDir())
208+ t.Setenv(envRemoteTransport, string(ports.RemoteTransportStdio))
209+
210+ observer := ports.NewSerializedRuntimeObserver(runtimeObserverFunc(func(ports.RuntimeMark) {}), 1)
211+ originalTrace := newPerformanceTrace
212+ traceCalls := 0
213+ newPerformanceTrace = func(ports.Clock) (ports.SerializedRuntimeObserver, io.Closer, error) {
214+ traceCalls++
215+ return observer, &runtimeTraceCloser{reporter: observer}, nil
216+ }
217+ t.Cleanup(func() { newPerformanceTrace = originalTrace })
218+
219+ factory := portsmocks.NewMockRemoteDialerFactory(t)
220+ factory.EXPECT().DialerForRemote("remote.example", "work", ports.RemoteTransportStdio, mock.Anything).Return(namedDialer{name: "remote"}, nil)
221+ originalFactory := newRemoteDialerFactoryWithRuntimeObserver
222+ factoryCalls := 0
223+ newRemoteDialerFactoryWithRuntimeObserver = func(got ports.SerializedRuntimeObserver) ports.RemoteDialerFactory {
224+ factoryCalls++
225+ if got != observer {
226+ t.Fatalf("remote transport observer = %v, want process observer %v", got, observer)
171227 }
228+ return factory
229+ }
230+ t.Cleanup(func() { newRemoteDialerFactoryWithRuntimeObserver = originalFactory })
231+
232+ originalRunClient := runClientWithRuntimeObserver
233+ runClientWithRuntimeObserver = func(_ context.Context, gotDialer ports.Dialer, _ ports.Terminal, _ ports.Clock, _ uint8, _ string, _ bool, _ ports.ClipboardReader, _ *slog.Logger, got ports.SerializedRuntimeObserver) error {
234+ if got != observer {
235+ t.Fatalf("client transport observer = %v, want process observer %v", got, observer)
236+ }
237+ if gotDialer != (namedDialer{name: "remote"}) {
238+ t.Fatalf("remote transport dialer = %v, want configured factory dialer", gotDialer)
239+ }
240+ return nil
241+ }
242+ t.Cleanup(func() { runClientWithRuntimeObserver = originalRunClient })
243+
244+ if err := runAttach(context.Background(), ports.IntentAttach, "work", "remote.example"); err != nil {
245+ t.Fatal(err)
172246 }
173- if strings.Contains(text, "WithRuntimeObserver(clock") || strings.Contains(text, "WithRuntimeObserver(clk") {
174- t.Error("consumer observer configuration must not accept a clock")
247+ if traceCalls != 1 || factoryCalls != 1 {
248+ t.Fatalf("trace/factory calls = %d/%d, want exactly one observer and one transport factory", traceCalls, factoryCalls)
175249 }
176250 }
@@ -6,6 +6,7 @@ import (
6 "encoding/json"6 "encoding/json"
7 "errors"7 "errors"
8 "io"8 "io"
9+ "log/slog"
9 "os"10 "os"
10 "path/filepath"11 "path/filepath"
11 "strconv"12 "strconv"
@@ -15,6 +16,7 @@ import (
15 16
16 "github.com/bnema/vev/internal/ports"17 "github.com/bnema/vev/internal/ports"
17 portsmocks "github.com/bnema/vev/internal/ports/mocks"18 portsmocks "github.com/bnema/vev/internal/ports/mocks"
19+ "github.com/stretchr/testify/mock"
18 )20 )
19 21
20 func TestPerformanceTraceUsesHarnessProcessMapping(t *testing.T) {22 func TestPerformanceTraceUsesHarnessProcessMapping(t *testing.T) {
@@ -46,7 +48,11 @@ func TestPerformanceTraceUsesHarnessProcessMapping(t *testing.T) {
46 if err != nil {48 if err != nil {
47 t.Fatal(err)49 t.Fatal(err)
48 }50 }
49- defer f.Close()51+ defer func() {
52+ if err := f.Close(); err != nil {
53+ t.Errorf("close trace file: %v", err)
54+ }
55+ }()
50 var mark struct {56 var mark struct {
51 ProcessID string `json:"process_id"`57 ProcessID string `json:"process_id"`
52 Scenario string `json:"scenario"`58 Scenario string `json:"scenario"`
@@ -63,6 +69,48 @@ func TestPerformanceTraceUsesHarnessProcessMapping(t *testing.T) {
63 }69 }
64 }70 }
65 71
72+func TestPerformanceTraceWithFactoriesCreatesOneConfiguredObserver(t *testing.T) {
73+ t.Setenv("VEV_PERF_TRACE", "trace.jsonl")
74+ t.Setenv("VEV_PERF_PROCESS_ID", "client-01")
75+ t.Setenv("VEV_PERF_SCENARIO", "lossy-remote")
76+ t.Setenv("VEV_PERF_RUN", "7")
77+
78+ clock := portsmocks.NewMockClock(t)
79+ sink := portsmocks.NewMockRuntimeObserver(t)
80+ sink.EXPECT().ObserveRuntime(mock.MatchedBy(func(mark ports.RuntimeMark) bool {
81+ return mark.Component == "client" && mark.Scenario == "lossy-remote" && mark.Run == 7 &&
82+ mark.Sequence != 0 && mark.RequestID != 0 && mark.Epoch != 0 && mark.Kind == ports.RuntimeEmitEnd
83+ })).Once()
84+
85+ var sinkCalls, correlationCalls int
86+ observer, closer, err := performanceTraceWithFactories(clock,
87+ func(path string, gotClock ports.Clock, processID string) (ports.RuntimeObserver, io.Closer, error) {
88+ sinkCalls++
89+ if path != "trace.jsonl" || gotClock != clock || processID != "client-01" {
90+ t.Fatalf("trace sink config = (%q, %v, %q)", path, gotClock, processID)
91+ }
92+ return sink, errorCloser{}, nil
93+ },
94+ func(got ports.RuntimeObserver, inputs ports.RuntimeCorrelationInputs) (ports.RuntimeObserver, error) {
95+ correlationCalls++
96+ if got != sink || inputs != (ports.RuntimeCorrelationInputs{Scenario: "lossy-remote", Run: 7}) {
97+ t.Fatalf("trace correlation config = (%v, %+v)", got, inputs)
98+ }
99+ return ports.NewRuntimeCorrelationObserver(got, inputs)
100+ },
101+ )
102+ if err != nil {
103+ t.Fatal(err)
104+ }
105+ if sinkCalls != 1 || correlationCalls != 1 || observer == nil || closer == nil {
106+ t.Fatalf("trace setup = sink calls %d, correlation calls %d, observer %v, closer %v; want one configured observer", sinkCalls, correlationCalls, observer, closer)
107+ }
108+ observer.ObserveRuntime(ports.NewRuntimeMark("client", ports.RuntimeEmitEnd, 0, true))
109+ if err := closer.Close(); err != nil {
110+ t.Fatal(err)
111+ }
112+}
113+
66 func TestPerformanceTraceSetupRollbackJoinsCloseError(t *testing.T) {114 func TestPerformanceTraceSetupRollbackJoinsCloseError(t *testing.T) {
67 setupErr := errors.New("correlation setup failed")115 setupErr := errors.New("correlation setup failed")
68 closeErr := errors.New("rollback close failed")116 closeErr := errors.New("rollback close failed")
@@ -154,23 +202,49 @@ type runtimeObserverFunc func(ports.RuntimeMark)
154 202
155 func (f runtimeObserverFunc) ObserveRuntime(mark ports.RuntimeMark) { f(mark) }203 func (f runtimeObserverFunc) ObserveRuntime(mark ports.RuntimeMark) { f(mark) }
156 204
157-func TestPerformanceTraceAppWiresOneObserverPerProcess(t *testing.T) {205+func TestRunAttachPropagatesOneObserverToRemoteTransportFactory(t *testing.T) {
158- source, err := os.ReadFile("run.go")206+ t.Setenv("XDG_STATE_HOME", t.TempDir())
159- if err != nil {207+ t.Setenv("XDG_RUNTIME_DIR", t.TempDir())
160- t.Fatalf("ReadFile(run.go): %v", err)208+ t.Setenv(envRemoteTransport, string(ports.RemoteTransportStdio))
161- }209+
162- text := string(source)210+ observer := ports.NewSerializedRuntimeObserver(runtimeObserverFunc(func(ports.RuntimeMark) {}), 1)
163- for _, required := range []string{211+ originalTrace := newPerformanceTrace
164- "VEV_PERF_TRACE",212+ traceCalls := 0
165- "VEV_PERF_PROCESS_ID",213+ newPerformanceTrace = func(ports.Clock) (ports.SerializedRuntimeObserver, io.Closer, error) {
166- "observability.NewJSONL",214+ traceCalls++
167- "WithRuntimeObserver",215+ return observer, &runtimeTraceCloser{reporter: observer}, nil
168- } {216+ }
169- if !strings.Contains(text, required) {217+ t.Cleanup(func() { newPerformanceTrace = originalTrace })
170- t.Errorf("runtime trace wiring missing %q", required)218+
219+ factory := portsmocks.NewMockRemoteDialerFactory(t)
220+ factory.EXPECT().DialerForRemote("remote.example", "work", ports.RemoteTransportStdio, mock.Anything).Return(namedDialer{name: "remote"}, nil)
221+ originalFactory := newRemoteDialerFactoryWithRuntimeObserver
222+ factoryCalls := 0
223+ newRemoteDialerFactoryWithRuntimeObserver = func(got ports.SerializedRuntimeObserver) ports.RemoteDialerFactory {
224+ factoryCalls++
225+ if got != observer {
226+ t.Fatalf("remote transport observer = %v, want process observer %v", got, observer)
171 }227 }
228+ return factory
229+ }
230+ t.Cleanup(func() { newRemoteDialerFactoryWithRuntimeObserver = originalFactory })
231+
232+ originalRunClient := runClientWithRuntimeObserver
233+ runClientWithRuntimeObserver = func(_ context.Context, gotDialer ports.Dialer, _ ports.Terminal, _ ports.Clock, _ uint8, _ string, _ bool, _ ports.ClipboardReader, _ *slog.Logger, got ports.SerializedRuntimeObserver) error {
234+ if got != observer {
235+ t.Fatalf("client transport observer = %v, want process observer %v", got, observer)
236+ }
237+ if gotDialer != (namedDialer{name: "remote"}) {
238+ t.Fatalf("remote transport dialer = %v, want configured factory dialer", gotDialer)
239+ }
240+ return nil
241+ }
242+ t.Cleanup(func() { runClientWithRuntimeObserver = originalRunClient })
243+
244+ if err := runAttach(context.Background(), ports.IntentAttach, "work", "remote.example"); err != nil {
245+ t.Fatal(err)
172 }246 }
173- if strings.Contains(text, "WithRuntimeObserver(clock") || strings.Contains(text, "WithRuntimeObserver(clk") {247+ if traceCalls != 1 || factoryCalls != 1 {
174- t.Error("consumer observer configuration must not accept a clock")248+ t.Fatalf("trace/factory calls = %d/%d, want exactly one observer and one transport factory", traceCalls, factoryCalls)
175 }249 }
176 }250 }
M internal/ports/ports.go +3 -1
@@ -40,7 +40,9 @@ type Terminal interface {
4040 Flush() error
4141 }
4242
43-// Transport is a framed message channel over a single connection.
43+// Transport is a framed message channel over a single connection. Close must
44+// be safe to call concurrently with Send and Recv, and must unblock active
45+// Send and Recv calls.
4446 type Transport interface {
4547 Send(Frame) error
4648 Recv() (Frame, error) // blocking; io.EOF on close
@@ -40,7 +40,9 @@ type Terminal interface {
40 Flush() error40 Flush() error
41 }41 }
42 42
43-// Transport is a framed message channel over a single connection.43+// Transport is a framed message channel over a single connection. Close must
44+// be safe to call concurrently with Send and Recv, and must unblock active
45+// Send and Recv calls.
44 type Transport interface {46 type Transport interface {
45 Send(Frame) error47 Send(Frame) error
46 Recv() (Frame, error) // blocking; io.EOF on close48 Recv() (Frame, error) // blocking; io.EOF on close
M internal/usecase/client/client.go +64 -9
@@ -102,6 +102,10 @@ const stdinBufSize = 4096
102102 // goroutine, decoupling the input/resize pumps from transport back-pressure.
103103 const sendQueueDepth = 64
104104
105+// preWelcomeTimeout bounds each blocking operation before the daemon has
106+// accepted the attach. Mosh uses the same 15-second startup budget.
107+const preWelcomeTimeout = 15 * time.Second
108+
105109 var reconnectSleep = sleepReconnect
106110 var reconnectSleepWithResize = sleepReconnectWithResizeEvents
107111
@@ -270,7 +274,9 @@ func run(ctx context.Context, dialer ports.Dialer, term ports.Terminal, clk port
270274 if result.welcomed {
271275 backoff = defaultReconnectBackoff.initial
272276 }
273- _ = transport.Close()
277+ if !result.transportClosed {
278+ _ = transport.Close()
279+ }
274280 if result.resumeToken != 0 {
275281 resumeToken = result.resumeToken
276282 }
@@ -350,10 +356,52 @@ func (d singleTransportDialer) Dial(context.Context) (ports.Transport, error) {
350356 }
351357
352358 type attachResult struct {
353- resumeToken uint64
354- sessionName string
355- welcomed bool
356- err error
359+ resumeToken uint64
360+ sessionName string
361+ welcomed bool
362+ transportClosed bool
363+ err error
364+}
365+
366+// boundedPreWelcome runs one blocking handshake operation. Transport methods
367+// do not accept a context, so cancellation and expiry close this attempt's
368+// transport to interrupt the call, then wait for its goroutine to exit.
369+func boundedPreWelcome(ctx context.Context, clk ports.Clock, transport ports.Transport, operation func() error) (bool, error) {
370+ if err := ctx.Err(); err != nil {
371+ _ = transport.Close()
372+ return true, err
373+ }
374+
375+ timer := clk.NewTimer(preWelcomeTimeout)
376+ defer timer.Stop()
377+ completed := make(chan error, 1)
378+ go func() { completed <- operation() }()
379+
380+ select {
381+ case err := <-completed:
382+ // Prefer a cancellation/expiry observed concurrently with completion.
383+ // That event owns the transport lifetime even though the operation has
384+ // already returned by the time we observe it here.
385+ if ctx.Err() != nil {
386+ _ = transport.Close()
387+ return true, ctx.Err()
388+ }
389+ select {
390+ case <-timer.C():
391+ _ = transport.Close()
392+ return true, context.DeadlineExceeded
393+ default:
394+ return false, err
395+ }
396+ case <-ctx.Done():
397+ _ = transport.Close()
398+ <-completed
399+ return true, ctx.Err()
400+ case <-timer.C():
401+ _ = transport.Close()
402+ <-completed
403+ return true, context.DeadlineExceeded
404+ }
357405 }
358406
359407 // DetectTrueColor reports whether TERM/COLORTERM advertise direct color support.
@@ -403,15 +451,22 @@ func attachOnce(ctx context.Context, transport ports.Transport, term ports.Termi
403451 TrueColor: trueColor,
404452 MaxOutputInFlight: requestedOutputWindow(transport),
405453 }
406- if err := transport.Send(ports.Frame{Type: ports.MsgHello, Payload: ports.MarshalHello(hello)}); err != nil {
407- return attachResult{err: fmt.Errorf("vev: sending hello: %w", err)}
454+ if closed, err := boundedPreWelcome(ctx, clk, transport, func() error {
455+ return transport.Send(ports.Frame{Type: ports.MsgHello, Payload: ports.MarshalHello(hello)})
456+ }); err != nil {
457+ return attachResult{transportClosed: closed, err: fmt.Errorf("vev: sending hello: %w", err)}
408458 }
409459 ms.helloSent = true
410460
411461 // 2. Await Welcome or a typed rejection.
412- reply, err := transport.Recv()
462+ var reply ports.Frame
463+ closed, err := boundedPreWelcome(ctx, clk, transport, func() error {
464+ var recvErr error
465+ reply, recvErr = transport.Recv()
466+ return recvErr
467+ })
413468 if err != nil {
414- return attachResult{err: fmt.Errorf("vev: awaiting welcome: %w", err)}
469+ return attachResult{transportClosed: closed, err: fmt.Errorf("vev: awaiting welcome: %w", err)}
415470 }
416471 result := func(welcomed bool, err error) attachResult {
417472 return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: welcomed, err: err}
@@ -102,6 +102,10 @@ const stdinBufSize = 4096
102 // goroutine, decoupling the input/resize pumps from transport back-pressure.102 // goroutine, decoupling the input/resize pumps from transport back-pressure.
103 const sendQueueDepth = 64103 const sendQueueDepth = 64
104 104
105+// preWelcomeTimeout bounds each blocking operation before the daemon has
106+// accepted the attach. Mosh uses the same 15-second startup budget.
107+const preWelcomeTimeout = 15 * time.Second
108+
105 var reconnectSleep = sleepReconnect109 var reconnectSleep = sleepReconnect
106 var reconnectSleepWithResize = sleepReconnectWithResizeEvents110 var reconnectSleepWithResize = sleepReconnectWithResizeEvents
107 111
@@ -270,7 +274,9 @@ func run(ctx context.Context, dialer ports.Dialer, term ports.Terminal, clk port
270 if result.welcomed {274 if result.welcomed {
271 backoff = defaultReconnectBackoff.initial275 backoff = defaultReconnectBackoff.initial
272 }276 }
273- _ = transport.Close()277+ if !result.transportClosed {
278+ _ = transport.Close()
279+ }
274 if result.resumeToken != 0 {280 if result.resumeToken != 0 {
275 resumeToken = result.resumeToken281 resumeToken = result.resumeToken
276 }282 }
@@ -350,10 +356,52 @@ func (d singleTransportDialer) Dial(context.Context) (ports.Transport, error) {
350 }356 }
351 357
352 type attachResult struct {358 type attachResult struct {
353- resumeToken uint64359+ resumeToken uint64
354- sessionName string360+ sessionName string
355- welcomed bool361+ welcomed bool
356- err error362+ transportClosed bool
363+ err error
364+}
365+
366+// boundedPreWelcome runs one blocking handshake operation. Transport methods
367+// do not accept a context, so cancellation and expiry close this attempt's
368+// transport to interrupt the call, then wait for its goroutine to exit.
369+func boundedPreWelcome(ctx context.Context, clk ports.Clock, transport ports.Transport, operation func() error) (bool, error) {
370+ if err := ctx.Err(); err != nil {
371+ _ = transport.Close()
372+ return true, err
373+ }
374+
375+ timer := clk.NewTimer(preWelcomeTimeout)
376+ defer timer.Stop()
377+ completed := make(chan error, 1)
378+ go func() { completed <- operation() }()
379+
380+ select {
381+ case err := <-completed:
382+ // Prefer a cancellation/expiry observed concurrently with completion.
383+ // That event owns the transport lifetime even though the operation has
384+ // already returned by the time we observe it here.
385+ if ctx.Err() != nil {
386+ _ = transport.Close()
387+ return true, ctx.Err()
388+ }
389+ select {
390+ case <-timer.C():
391+ _ = transport.Close()
392+ return true, context.DeadlineExceeded
393+ default:
394+ return false, err
395+ }
396+ case <-ctx.Done():
397+ _ = transport.Close()
398+ <-completed
399+ return true, ctx.Err()
400+ case <-timer.C():
401+ _ = transport.Close()
402+ <-completed
403+ return true, context.DeadlineExceeded
404+ }
357 }405 }
358 406
359 // DetectTrueColor reports whether TERM/COLORTERM advertise direct color support.407 // DetectTrueColor reports whether TERM/COLORTERM advertise direct color support.
@@ -403,15 +451,22 @@ func attachOnce(ctx context.Context, transport ports.Transport, term ports.Termi
403 TrueColor: trueColor,451 TrueColor: trueColor,
404 MaxOutputInFlight: requestedOutputWindow(transport),452 MaxOutputInFlight: requestedOutputWindow(transport),
405 }453 }
406- if err := transport.Send(ports.Frame{Type: ports.MsgHello, Payload: ports.MarshalHello(hello)}); err != nil {454+ if closed, err := boundedPreWelcome(ctx, clk, transport, func() error {
407- return attachResult{err: fmt.Errorf("vev: sending hello: %w", err)}455+ return transport.Send(ports.Frame{Type: ports.MsgHello, Payload: ports.MarshalHello(hello)})
456+ }); err != nil {
457+ return attachResult{transportClosed: closed, err: fmt.Errorf("vev: sending hello: %w", err)}
408 }458 }
409 ms.helloSent = true459 ms.helloSent = true
410 460
411 // 2. Await Welcome or a typed rejection.461 // 2. Await Welcome or a typed rejection.
412- reply, err := transport.Recv()462+ var reply ports.Frame
463+ closed, err := boundedPreWelcome(ctx, clk, transport, func() error {
464+ var recvErr error
465+ reply, recvErr = transport.Recv()
466+ return recvErr
467+ })
413 if err != nil {468 if err != nil {
414- return attachResult{err: fmt.Errorf("vev: awaiting welcome: %w", err)}469+ return attachResult{transportClosed: closed, err: fmt.Errorf("vev: awaiting welcome: %w", err)}
415 }470 }
416 result := func(welcomed bool, err error) attachResult {471 result := func(welcomed bool, err error) attachResult {
417 return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: welcomed, err: err}472 return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: welcomed, err: err}
M internal/usecase/client/client_test.go +114 -0
@@ -155,6 +155,120 @@ func isType(typ ports.MsgType) any {
155155 return mock.MatchedBy(func(f ports.Frame) bool { return f.Type == typ })
156156 }
157157
158+const testPreWelcomeTimeout = 15 * time.Second
159+
160+func newHandshakeClock(t *testing.T, timerCount int) (*portsmocks.MockClock, <-chan chan time.Time) {
161+ t.Helper()
162+ created := make(chan chan time.Time, timerCount)
163+ clk := portsmocks.NewMockClock(t)
164+ clk.EXPECT().NewTimer(testPreWelcomeTimeout).RunAndReturn(func(time.Duration) ports.Timer {
165+ timerC := make(chan time.Time, 1)
166+ timer := portsmocks.NewMockTimer(t)
167+ timer.EXPECT().C().Return((<-chan time.Time)(timerC)).Maybe()
168+ timer.EXPECT().Stop().Return(true).Once()
169+ created <- timerC
170+ return timer
171+ }).Times(timerCount)
172+ return clk, created
173+}
174+
175+func TestRunBoundsPreWelcomeOperations(t *testing.T) {
176+ tests := []struct {
177+ name string
178+ phase string
179+ end string
180+ want error
181+ }{
182+ {name: "cancel while sending hello", phase: "send", end: "cancel", want: context.Canceled},
183+ {name: "timeout while sending hello", phase: "send", end: "timeout", want: context.DeadlineExceeded},
184+ {name: "cancel while receiving welcome", phase: "recv", end: "cancel", want: context.Canceled},
185+ {name: "timeout while receiving welcome", phase: "recv", end: "timeout", want: context.DeadlineExceeded},
186+ }
187+
188+ for _, tt := range tests {
189+ t.Run(tt.name, func(t *testing.T) {
190+ ctx, cancel := context.WithCancel(context.Background())
191+ defer cancel()
192+ timerCount := 1
193+ if tt.phase == "recv" {
194+ timerCount = 2
195+ }
196+ clk, createdTimers := newHandshakeClock(t, timerCount)
197+
198+ term := portsmocks.NewMockTerminal(t)
199+ term.EXPECT().Size().Return(domain.Size{Cols: 80, Rows: 24}, nil).Once()
200+ // No EnterRaw expectation: it must not run before Welcome.
201+
202+ started := make(chan struct{})
203+ closed := make(chan struct{})
204+ operationExited := make(chan struct{})
205+ tr := portsmocks.NewMockTransport(t)
206+ tr.EXPECT().Close().Run(func() { close(closed) }).Return(nil).Once()
207+
208+ block := func() error {
209+ close(started)
210+ <-closed
211+ close(operationExited)
212+ return io.ErrClosedPipe
213+ }
214+ if tt.phase == "send" {
215+ tr.EXPECT().Send(isType(ports.MsgHello)).RunAndReturn(func(ports.Frame) error {
216+ return block()
217+ }).Once()
218+ } else {
219+ tr.EXPECT().Send(isType(ports.MsgHello)).Return(nil).Once()
220+ tr.EXPECT().Recv().RunAndReturn(func() (ports.Frame, error) {
221+ return ports.Frame{}, block()
222+ }).Once()
223+ }
224+ dialer := portsmocks.NewMockDialer(t)
225+ dialer.EXPECT().Dial(mock.Anything).Return(tr, nil).Once()
226+
227+ errCh := make(chan error, 1)
228+ go func() {
229+ errCh <- client.Run(ctx, dialer, term, clk, ports.IntentAttach, "main", false, nil, slog.New(slog.DiscardHandler))
230+ }()
231+
232+ var timerC chan time.Time
233+ select {
234+ case timerC = <-createdTimers:
235+ case <-time.After(time.Second):
236+ t.Fatal("pre-Welcome timer was not created")
237+ }
238+ select {
239+ case <-started:
240+ case <-time.After(time.Second):
241+ t.Fatal("pre-Welcome operation did not start")
242+ }
243+ if tt.phase == "recv" {
244+ select {
245+ case timerC = <-createdTimers:
246+ case <-time.After(time.Second):
247+ t.Fatal("Welcome timer was not created")
248+ }
249+ }
250+
251+ if tt.end == "cancel" {
252+ cancel()
253+ } else {
254+ timerC <- time.Time{}
255+ }
256+
257+ select {
258+ case err := <-errCh:
259+ require.ErrorIs(t, err, tt.want)
260+ case <-time.After(time.Second):
261+ t.Fatal("Run did not return after pre-Welcome cancellation")
262+ }
263+ select {
264+ case <-operationExited:
265+ case <-time.After(time.Second):
266+ t.Fatal("pre-Welcome operation leaked after Run returned")
267+ }
268+ })
269+ }
270+}
271+
158272 func TestAttachHelloIncludesTrueColor(t *testing.T) {
159273 t.Setenv("TERM", "xterm-256color")
160274 t.Setenv("COLORTERM", "truecolor")
@@ -155,6 +155,120 @@ func isType(typ ports.MsgType) any {
155 return mock.MatchedBy(func(f ports.Frame) bool { return f.Type == typ })155 return mock.MatchedBy(func(f ports.Frame) bool { return f.Type == typ })
156 }156 }
157 157
158+const testPreWelcomeTimeout = 15 * time.Second
159+
160+func newHandshakeClock(t *testing.T, timerCount int) (*portsmocks.MockClock, <-chan chan time.Time) {
161+ t.Helper()
162+ created := make(chan chan time.Time, timerCount)
163+ clk := portsmocks.NewMockClock(t)
164+ clk.EXPECT().NewTimer(testPreWelcomeTimeout).RunAndReturn(func(time.Duration) ports.Timer {
165+ timerC := make(chan time.Time, 1)
166+ timer := portsmocks.NewMockTimer(t)
167+ timer.EXPECT().C().Return((<-chan time.Time)(timerC)).Maybe()
168+ timer.EXPECT().Stop().Return(true).Once()
169+ created <- timerC
170+ return timer
171+ }).Times(timerCount)
172+ return clk, created
173+}
174+
175+func TestRunBoundsPreWelcomeOperations(t *testing.T) {
176+ tests := []struct {
177+ name string
178+ phase string
179+ end string
180+ want error
181+ }{
182+ {name: "cancel while sending hello", phase: "send", end: "cancel", want: context.Canceled},
183+ {name: "timeout while sending hello", phase: "send", end: "timeout", want: context.DeadlineExceeded},
184+ {name: "cancel while receiving welcome", phase: "recv", end: "cancel", want: context.Canceled},
185+ {name: "timeout while receiving welcome", phase: "recv", end: "timeout", want: context.DeadlineExceeded},
186+ }
187+
188+ for _, tt := range tests {
189+ t.Run(tt.name, func(t *testing.T) {
190+ ctx, cancel := context.WithCancel(context.Background())
191+ defer cancel()
192+ timerCount := 1
193+ if tt.phase == "recv" {
194+ timerCount = 2
195+ }
196+ clk, createdTimers := newHandshakeClock(t, timerCount)
197+
198+ term := portsmocks.NewMockTerminal(t)
199+ term.EXPECT().Size().Return(domain.Size{Cols: 80, Rows: 24}, nil).Once()
200+ // No EnterRaw expectation: it must not run before Welcome.
201+
202+ started := make(chan struct{})
203+ closed := make(chan struct{})
204+ operationExited := make(chan struct{})
205+ tr := portsmocks.NewMockTransport(t)
206+ tr.EXPECT().Close().Run(func() { close(closed) }).Return(nil).Once()
207+
208+ block := func() error {
209+ close(started)
210+ <-closed
211+ close(operationExited)
212+ return io.ErrClosedPipe
213+ }
214+ if tt.phase == "send" {
215+ tr.EXPECT().Send(isType(ports.MsgHello)).RunAndReturn(func(ports.Frame) error {
216+ return block()
217+ }).Once()
218+ } else {
219+ tr.EXPECT().Send(isType(ports.MsgHello)).Return(nil).Once()
220+ tr.EXPECT().Recv().RunAndReturn(func() (ports.Frame, error) {
221+ return ports.Frame{}, block()
222+ }).Once()
223+ }
224+ dialer := portsmocks.NewMockDialer(t)
225+ dialer.EXPECT().Dial(mock.Anything).Return(tr, nil).Once()
226+
227+ errCh := make(chan error, 1)
228+ go func() {
229+ errCh <- client.Run(ctx, dialer, term, clk, ports.IntentAttach, "main", false, nil, slog.New(slog.DiscardHandler))
230+ }()
231+
232+ var timerC chan time.Time
233+ select {
234+ case timerC = <-createdTimers:
235+ case <-time.After(time.Second):
236+ t.Fatal("pre-Welcome timer was not created")
237+ }
238+ select {
239+ case <-started:
240+ case <-time.After(time.Second):
241+ t.Fatal("pre-Welcome operation did not start")
242+ }
243+ if tt.phase == "recv" {
244+ select {
245+ case timerC = <-createdTimers:
246+ case <-time.After(time.Second):
247+ t.Fatal("Welcome timer was not created")
248+ }
249+ }
250+
251+ if tt.end == "cancel" {
252+ cancel()
253+ } else {
254+ timerC <- time.Time{}
255+ }
256+
257+ select {
258+ case err := <-errCh:
259+ require.ErrorIs(t, err, tt.want)
260+ case <-time.After(time.Second):
261+ t.Fatal("Run did not return after pre-Welcome cancellation")
262+ }
263+ select {
264+ case <-operationExited:
265+ case <-time.After(time.Second):
266+ t.Fatal("pre-Welcome operation leaked after Run returned")
267+ }
268+ })
269+ }
270+}
271+
158 func TestAttachHelloIncludesTrueColor(t *testing.T) {272 func TestAttachHelloIncludesTrueColor(t *testing.T) {
159 t.Setenv("TERM", "xterm-256color")273 t.Setenv("TERM", "xterm-256color")
160 t.Setenv("COLORTERM", "truecolor")274 t.Setenv("COLORTERM", "truecolor")
M internal/usecase/client/reconnect_toast_test.go +18 -6
@@ -179,6 +179,18 @@ func reconnectToastHelloFromSend(t *testing.T, tr *reconnectToastRecordingTransp
179179 return hello
180180 }
181181
182+// newReconnectHandshakeClock supplies the non-firing timer needed by the
183+// bounded pre-Welcome handshake; reconnect sleeps are stubbed by these tests.
184+func newReconnectHandshakeClock(t *testing.T) *portsmocks.MockClock {
185+ t.Helper()
186+ clk := portsmocks.NewMockClock(t)
187+ timer := portsmocks.NewMockTimer(t)
188+ timer.EXPECT().C().Return((<-chan time.Time)(make(chan time.Time))).Maybe()
189+ timer.EXPECT().Stop().Return(true).Maybe()
190+ clk.EXPECT().NewTimer(preWelcomeTimeout).Return(timer).Maybe()
191+ return clk
192+}
193+
182194 func TestReconnectToastDegradedClearsModalAndProbingIsVisible(t *testing.T) {
183195 term := newReconnectToastTerminalHarness(t)
184196 defer term.closeInput()
@@ -190,7 +202,7 @@ func TestReconnectToastDegradedClearsModalAndProbingIsVisible(t *testing.T) {
190202 resultCh := make(chan attachResult, 1)
191203 ms := milestones{}
192204 go func() {
193- resultCh <- attachOnce(context.Background(), tr, term.term, portsmocks.NewMockClock(t), ports.IntentAttach, "main", 0, [16]byte{1}, &ms, func() error {
205+ resultCh <- attachOnce(context.Background(), tr, term.term, newReconnectHandshakeClock(t), ports.IntentAttach, "main", 0, [16]byte{1}, &ms, func() error {
194206 _, err := term.term.EnterRaw()
195207 return err
196208 }, func() { clears <- struct{}{} }, func(stage reconnectStage) {
@@ -229,7 +241,7 @@ func TestAttachOnceOfflineLinkEventReturnsReconnectableError(t *testing.T) {
229241 resultCh := make(chan attachResult, 1)
230242 ms := milestones{}
231243 go func() {
232- resultCh <- attachOnce(context.Background(), tr, term.term, portsmocks.NewMockClock(t), ports.IntentAttach, "main", 0, [16]byte{1}, &ms, func() error {
244+ resultCh <- attachOnce(context.Background(), tr, term.term, newReconnectHandshakeClock(t), ports.IntentAttach, "main", 0, [16]byte{1}, &ms, func() error {
233245 _, err := term.term.EnterRaw()
234246 return err
235247 }, func() {}, func(stage reconnectStage) {
@@ -400,7 +412,7 @@ func TestRemoteReconnectToastClearsFailedDrawBounds(t *testing.T) {
400412 }}
401413 dialer := &reconnectToastSequenceDialer{transports: []ports.Transport{tr}}
402414
403- err := Run(ctx, dialer, term.term, portsmocks.NewMockClock(t), ports.IntentAttach, "main", true, nil, slog.New(slog.DiscardHandler))
415+ err := Run(ctx, dialer, term.term, newReconnectHandshakeClock(t), ports.IntentAttach, "main", true, nil, slog.New(slog.DiscardHandler))
404416 require.ErrorIs(t, err, context.Canceled)
405417 require.True(t, out.failed)
406418 assertReconnectToastClearCoversBounds(t, out.String(), reconnectToastBoundsFor(term.size, reconnectStageMessage(reconnectStageSSH)))
@@ -435,7 +447,7 @@ func TestRemoteReconnectToastLifecycleWithWrappedTransportError(t *testing.T) {
435447 }}
436448 dialer := &reconnectToastSequenceDialer{transports: []ports.Transport{tr1, tr2}}
437449
438- err := Run(context.Background(), dialer, term.term, portsmocks.NewMockClock(t), ports.IntentAttach, "main", true, nil, slog.New(slog.DiscardHandler))
450+ err := Run(context.Background(), dialer, term.term, newReconnectHandshakeClock(t), ports.IntentAttach, "main", true, nil, slog.New(slog.DiscardHandler))
439451 require.NoError(t, err)
440452 require.True(t, tr1.closed)
441453 require.True(t, tr2.closed)
@@ -479,7 +491,7 @@ func TestRemoteEphemeralReconnectUsesAssignedSessionName(t *testing.T) {
479491 }}
480492 dialer := &reconnectToastSequenceDialer{transports: []ports.Transport{tr1, tr2}}
481493
482- err := Run(context.Background(), dialer, term.term, portsmocks.NewMockClock(t), ports.IntentEphemeral, "", true, nil, slog.New(slog.DiscardHandler))
494+ err := Run(context.Background(), dialer, term.term, newReconnectHandshakeClock(t), ports.IntentEphemeral, "", true, nil, slog.New(slog.DiscardHandler))
483495 require.NoError(t, err)
484496 require.Equal(t, 2, dialer.calls)
485497
@@ -564,7 +576,7 @@ func TestRemoteReconnectToastLifecycle(t *testing.T) {
564576 dialer := portsmocks.NewMockDialer(t)
565577 tt.configure(t, dialer)
566578
567- err := Run(ctx, dialer, term.term, portsmocks.NewMockClock(t), ports.IntentAttach, "main", true, nil, slog.New(slog.DiscardHandler))
579+ err := Run(ctx, dialer, term.term, newReconnectHandshakeClock(t), ports.IntentAttach, "main", true, nil, slog.New(slog.DiscardHandler))
568580 tt.wantErr(t, err)
569581 out := term.out.String()
570582 require.Contains(t, out, reconnectToastMessage)
@@ -179,6 +179,18 @@ func reconnectToastHelloFromSend(t *testing.T, tr *reconnectToastRecordingTransp
179 return hello179 return hello
180 }180 }
181 181
182+// newReconnectHandshakeClock supplies the non-firing timer needed by the
183+// bounded pre-Welcome handshake; reconnect sleeps are stubbed by these tests.
184+func newReconnectHandshakeClock(t *testing.T) *portsmocks.MockClock {
185+ t.Helper()
186+ clk := portsmocks.NewMockClock(t)
187+ timer := portsmocks.NewMockTimer(t)
188+ timer.EXPECT().C().Return((<-chan time.Time)(make(chan time.Time))).Maybe()
189+ timer.EXPECT().Stop().Return(true).Maybe()
190+ clk.EXPECT().NewTimer(preWelcomeTimeout).Return(timer).Maybe()
191+ return clk
192+}
193+
182 func TestReconnectToastDegradedClearsModalAndProbingIsVisible(t *testing.T) {194 func TestReconnectToastDegradedClearsModalAndProbingIsVisible(t *testing.T) {
183 term := newReconnectToastTerminalHarness(t)195 term := newReconnectToastTerminalHarness(t)
184 defer term.closeInput()196 defer term.closeInput()
@@ -190,7 +202,7 @@ func TestReconnectToastDegradedClearsModalAndProbingIsVisible(t *testing.T) {
190 resultCh := make(chan attachResult, 1)202 resultCh := make(chan attachResult, 1)
191 ms := milestones{}203 ms := milestones{}
192 go func() {204 go func() {
193- resultCh <- attachOnce(context.Background(), tr, term.term, portsmocks.NewMockClock(t), ports.IntentAttach, "main", 0, [16]byte{1}, &ms, func() error {205+ resultCh <- attachOnce(context.Background(), tr, term.term, newReconnectHandshakeClock(t), ports.IntentAttach, "main", 0, [16]byte{1}, &ms, func() error {
194 _, err := term.term.EnterRaw()206 _, err := term.term.EnterRaw()
195 return err207 return err
196 }, func() { clears <- struct{}{} }, func(stage reconnectStage) {208 }, func() { clears <- struct{}{} }, func(stage reconnectStage) {
@@ -229,7 +241,7 @@ func TestAttachOnceOfflineLinkEventReturnsReconnectableError(t *testing.T) {
229 resultCh := make(chan attachResult, 1)241 resultCh := make(chan attachResult, 1)
230 ms := milestones{}242 ms := milestones{}
231 go func() {243 go func() {
232- resultCh <- attachOnce(context.Background(), tr, term.term, portsmocks.NewMockClock(t), ports.IntentAttach, "main", 0, [16]byte{1}, &ms, func() error {244+ resultCh <- attachOnce(context.Background(), tr, term.term, newReconnectHandshakeClock(t), ports.IntentAttach, "main", 0, [16]byte{1}, &ms, func() error {
233 _, err := term.term.EnterRaw()245 _, err := term.term.EnterRaw()
234 return err246 return err
235 }, func() {}, func(stage reconnectStage) {247 }, func() {}, func(stage reconnectStage) {
@@ -400,7 +412,7 @@ func TestRemoteReconnectToastClearsFailedDrawBounds(t *testing.T) {
400 }}412 }}
401 dialer := &reconnectToastSequenceDialer{transports: []ports.Transport{tr}}413 dialer := &reconnectToastSequenceDialer{transports: []ports.Transport{tr}}
402 414
403- err := Run(ctx, dialer, term.term, portsmocks.NewMockClock(t), ports.IntentAttach, "main", true, nil, slog.New(slog.DiscardHandler))415+ err := Run(ctx, dialer, term.term, newReconnectHandshakeClock(t), ports.IntentAttach, "main", true, nil, slog.New(slog.DiscardHandler))
404 require.ErrorIs(t, err, context.Canceled)416 require.ErrorIs(t, err, context.Canceled)
405 require.True(t, out.failed)417 require.True(t, out.failed)
406 assertReconnectToastClearCoversBounds(t, out.String(), reconnectToastBoundsFor(term.size, reconnectStageMessage(reconnectStageSSH)))418 assertReconnectToastClearCoversBounds(t, out.String(), reconnectToastBoundsFor(term.size, reconnectStageMessage(reconnectStageSSH)))
@@ -435,7 +447,7 @@ func TestRemoteReconnectToastLifecycleWithWrappedTransportError(t *testing.T) {
435 }}447 }}
436 dialer := &reconnectToastSequenceDialer{transports: []ports.Transport{tr1, tr2}}448 dialer := &reconnectToastSequenceDialer{transports: []ports.Transport{tr1, tr2}}
437 449
438- err := Run(context.Background(), dialer, term.term, portsmocks.NewMockClock(t), ports.IntentAttach, "main", true, nil, slog.New(slog.DiscardHandler))450+ err := Run(context.Background(), dialer, term.term, newReconnectHandshakeClock(t), ports.IntentAttach, "main", true, nil, slog.New(slog.DiscardHandler))
439 require.NoError(t, err)451 require.NoError(t, err)
440 require.True(t, tr1.closed)452 require.True(t, tr1.closed)
441 require.True(t, tr2.closed)453 require.True(t, tr2.closed)
@@ -479,7 +491,7 @@ func TestRemoteEphemeralReconnectUsesAssignedSessionName(t *testing.T) {
479 }}491 }}
480 dialer := &reconnectToastSequenceDialer{transports: []ports.Transport{tr1, tr2}}492 dialer := &reconnectToastSequenceDialer{transports: []ports.Transport{tr1, tr2}}
481 493
482- err := Run(context.Background(), dialer, term.term, portsmocks.NewMockClock(t), ports.IntentEphemeral, "", true, nil, slog.New(slog.DiscardHandler))494+ err := Run(context.Background(), dialer, term.term, newReconnectHandshakeClock(t), ports.IntentEphemeral, "", true, nil, slog.New(slog.DiscardHandler))
483 require.NoError(t, err)495 require.NoError(t, err)
484 require.Equal(t, 2, dialer.calls)496 require.Equal(t, 2, dialer.calls)
485 497
@@ -564,7 +576,7 @@ func TestRemoteReconnectToastLifecycle(t *testing.T) {
564 dialer := portsmocks.NewMockDialer(t)576 dialer := portsmocks.NewMockDialer(t)
565 tt.configure(t, dialer)577 tt.configure(t, dialer)
566 578
567- err := Run(ctx, dialer, term.term, portsmocks.NewMockClock(t), ports.IntentAttach, "main", true, nil, slog.New(slog.DiscardHandler))579+ err := Run(ctx, dialer, term.term, newReconnectHandshakeClock(t), ports.IntentAttach, "main", true, nil, slog.New(slog.DiscardHandler))
568 tt.wantErr(t, err)580 tt.wantErr(t, err)
569 out := term.out.String()581 out := term.out.String()
570 require.Contains(t, out, reconnectToastMessage)582 require.Contains(t, out, reconnectToastMessage)
M internal/usecase/daemon/client.go +44 -8
@@ -174,6 +174,23 @@ func (ac *attachedClient) replaceTransport(tr ports.Transport) {
174174 ac.linkMu.Unlock()
175175 }
176176
177+// revokeTransport removes tr only if it is still this attachment's current
178+// link. It returns the revoked transport, so asynchronous retirement can
179+// close exactly that link without touching a later resume/replacement.
180+func (ac *attachedClient) revokeTransport(tr ports.Transport) ports.Transport {
181+ if tr == nil {
182+ return nil
183+ }
184+ ac.linkMu.Lock()
185+ defer ac.linkMu.Unlock()
186+ if ac.tr != tr {
187+ return nil
188+ }
189+ ac.tr = nil
190+ ac.transportIncarnation++
191+ return tr
192+}
193+
177194 // transportSnapshot binds a send to one concrete link lifetime. Callers must
178195 // revalidate it after acquiring sendMu before writing to that link.
179196 func (ac *attachedClient) transportSnapshot() transportSnapshot {
@@ -509,17 +526,36 @@ func (d *Daemon) resetScreenDefaultColors(sess *session) {
509526 d.applyHostTheme(sess, nil, d.effectiveTheme(themeui.Theme{}), true)
510527 }
511528
512-func (d *Daemon) detachReplacedClient(old *attachedClient) {
529+// retireReplacedClient owns old-link retirement and obsolete render-worker
530+// joins outside the replacement handshake. If a render already owns sendMu,
531+// close immediately interrupts it; no Detached frame can safely overtake that
532+// render. Otherwise the old link gets its bounded protocol notice first.
533+func (d *Daemon) retireReplacedClient(old *attachedClient, cleanup renderLifecycleCleanup) {
513534 if old == nil {
535+ d.attachmentCleanupWg.Go(cleanup.finish)
514536 return
515537 }
516- d.unregisterPreview(old)
517- old.clearPreviousSession()
518- old.setSession(nil)
519- d.log.Info("client displaced")
520- // Async + bounded: a dead or wedged old client must not stall the new
521- // client's handshake.
522- d.notifyDetachedAsync(old, ports.ReasonDetach)
538+ oldTransport := old.transportSnapshot().transport
539+ blockedRender := !old.sendMu.TryLock()
540+ if !blockedRender {
541+ old.sendMu.Unlock()
542+ }
543+
544+ d.attachmentCleanupWg.Go(func() {
545+ if !blockedRender {
546+ // A healthy idle client observes the required replacement notice. Its
547+ // watchdog still bounds a peer that stops draining after this check.
548+ d.boundedSend(old, frameDetached(ports.ReasonDetach))
549+ }
550+ _ = old.closeCapturedTransport(old.revokeTransport(oldTransport))
551+ d.unregisterPreview(old)
552+ old.clearPreviousSession()
553+ old.setSession(nil)
554+ d.log.Info("client displaced")
555+ })
556+ // Join independently: the close above releases a blocked render while the
557+ // new attachment is already fully published.
558+ d.attachmentCleanupWg.Go(cleanup.finish)
523559 }
524560
525561 // firstPaint guarantees the freshly attached client sees the full screen: if
@@ -174,6 +174,23 @@ func (ac *attachedClient) replaceTransport(tr ports.Transport) {
174 ac.linkMu.Unlock()174 ac.linkMu.Unlock()
175 }175 }
176 176
177+// revokeTransport removes tr only if it is still this attachment's current
178+// link. It returns the revoked transport, so asynchronous retirement can
179+// close exactly that link without touching a later resume/replacement.
180+func (ac *attachedClient) revokeTransport(tr ports.Transport) ports.Transport {
181+ if tr == nil {
182+ return nil
183+ }
184+ ac.linkMu.Lock()
185+ defer ac.linkMu.Unlock()
186+ if ac.tr != tr {
187+ return nil
188+ }
189+ ac.tr = nil
190+ ac.transportIncarnation++
191+ return tr
192+}
193+
177 // transportSnapshot binds a send to one concrete link lifetime. Callers must194 // transportSnapshot binds a send to one concrete link lifetime. Callers must
178 // revalidate it after acquiring sendMu before writing to that link.195 // revalidate it after acquiring sendMu before writing to that link.
179 func (ac *attachedClient) transportSnapshot() transportSnapshot {196 func (ac *attachedClient) transportSnapshot() transportSnapshot {
@@ -509,17 +526,36 @@ func (d *Daemon) resetScreenDefaultColors(sess *session) {
509 d.applyHostTheme(sess, nil, d.effectiveTheme(themeui.Theme{}), true)526 d.applyHostTheme(sess, nil, d.effectiveTheme(themeui.Theme{}), true)
510 }527 }
511 528
512-func (d *Daemon) detachReplacedClient(old *attachedClient) {529+// retireReplacedClient owns old-link retirement and obsolete render-worker
530+// joins outside the replacement handshake. If a render already owns sendMu,
531+// close immediately interrupts it; no Detached frame can safely overtake that
532+// render. Otherwise the old link gets its bounded protocol notice first.
533+func (d *Daemon) retireReplacedClient(old *attachedClient, cleanup renderLifecycleCleanup) {
513 if old == nil {534 if old == nil {
535+ d.attachmentCleanupWg.Go(cleanup.finish)
514 return536 return
515 }537 }
516- d.unregisterPreview(old)538+ oldTransport := old.transportSnapshot().transport
517- old.clearPreviousSession()539+ blockedRender := !old.sendMu.TryLock()
518- old.setSession(nil)540+ if !blockedRender {
519- d.log.Info("client displaced")541+ old.sendMu.Unlock()
520- // Async + bounded: a dead or wedged old client must not stall the new542+ }
521- // client's handshake.543+
522- d.notifyDetachedAsync(old, ports.ReasonDetach)544+ d.attachmentCleanupWg.Go(func() {
545+ if !blockedRender {
546+ // A healthy idle client observes the required replacement notice. Its
547+ // watchdog still bounds a peer that stops draining after this check.
548+ d.boundedSend(old, frameDetached(ports.ReasonDetach))
549+ }
550+ _ = old.closeCapturedTransport(old.revokeTransport(oldTransport))
551+ d.unregisterPreview(old)
552+ old.clearPreviousSession()
553+ old.setSession(nil)
554+ d.log.Info("client displaced")
555+ })
556+ // Join independently: the close above releases a blocked render while the
557+ // new attachment is already fully published.
558+ d.attachmentCleanupWg.Go(cleanup.finish)
523 }559 }
524 560
525 // firstPaint guarantees the freshly attached client sees the full screen: if561 // firstPaint guarantees the freshly attached client sees the full screen: if
M internal/usecase/daemon/copymode_test.go +2 -20
@@ -401,24 +401,6 @@ func TestScrollbackEvictionFeedsCopyModeYank(t *testing.T) {
401401 t.Fatal("coordinator did not arm a controllable render timer")
402402 }
403403 }
404- awaitControlledOutput := func() ports.Frame {
405- deadline := time.NewTimer(2 * time.Second)
406- defer deadline.Stop()
407- for {
408- select {
409- case frame := <-sends:
410- if frame.Type == ports.MsgOutput {
411- return frame
412- }
413- t.Fatalf("unexpected frame type %d while advancing render clock", frame.Type)
414- case timer := <-clk.timers:
415- timer.ch <- time.Time{}
416- case <-deadline.C:
417- t.Fatal("controllable timers did not produce an output frame")
418- return ports.Frame{}
419- }
420- }
421- }
422404 var hg sync.WaitGroup
423405 hg.Go(func() { d.handleConn(tr) })
424406 awaitFrame(t, sends, ports.MsgWelcome)
@@ -453,9 +435,9 @@ func TestScrollbackEvictionFeedsCopyModeYank(t *testing.T) {
453435 ac := sess.client
454436 require.NotNil(t, ac)
455437 d.handleInput(sess, ac, []byte("\x1b "))
456- awaitControlledOutput()
438+ awaitCoordinatorOutput(t, sends, clk.timers, "while advancing render clock", "controllable timers did not produce an output frame")
457439 d.handleInput(sess, ac, []byte("VIS\r"))
458- awaitControlledOutput()
440+ awaitCoordinatorOutput(t, sends, clk.timers, "while advancing render clock", "controllable timers did not produce an output frame")
459441 d.handleInput(sess, ac, []byte{'g', ' ', 'G', 'y'})
460442
461443 var payload string
@@ -401,24 +401,6 @@ func TestScrollbackEvictionFeedsCopyModeYank(t *testing.T) {
401 t.Fatal("coordinator did not arm a controllable render timer")401 t.Fatal("coordinator did not arm a controllable render timer")
402 }402 }
403 }403 }
404- awaitControlledOutput := func() ports.Frame {
405- deadline := time.NewTimer(2 * time.Second)
406- defer deadline.Stop()
407- for {
408- select {
409- case frame := <-sends:
410- if frame.Type == ports.MsgOutput {
411- return frame
412- }
413- t.Fatalf("unexpected frame type %d while advancing render clock", frame.Type)
414- case timer := <-clk.timers:
415- timer.ch <- time.Time{}
416- case <-deadline.C:
417- t.Fatal("controllable timers did not produce an output frame")
418- return ports.Frame{}
419- }
420- }
421- }
422 var hg sync.WaitGroup404 var hg sync.WaitGroup
423 hg.Go(func() { d.handleConn(tr) })405 hg.Go(func() { d.handleConn(tr) })
424 awaitFrame(t, sends, ports.MsgWelcome)406 awaitFrame(t, sends, ports.MsgWelcome)
@@ -453,9 +435,9 @@ func TestScrollbackEvictionFeedsCopyModeYank(t *testing.T) {
453 ac := sess.client435 ac := sess.client
454 require.NotNil(t, ac)436 require.NotNil(t, ac)
455 d.handleInput(sess, ac, []byte("\x1b "))437 d.handleInput(sess, ac, []byte("\x1b "))
456- awaitControlledOutput()438+ awaitCoordinatorOutput(t, sends, clk.timers, "while advancing render clock", "controllable timers did not produce an output frame")
457 d.handleInput(sess, ac, []byte("VIS\r"))439 d.handleInput(sess, ac, []byte("VIS\r"))
458- awaitControlledOutput()440+ awaitCoordinatorOutput(t, sends, clk.timers, "while advancing render clock", "controllable timers did not produce an output frame")
459 d.handleInput(sess, ac, []byte{'g', ' ', 'G', 'y'})441 d.handleInput(sess, ac, []byte{'g', ' ', 'G', 'y'})
460 442
461 var payload string443 var payload string
M internal/usecase/daemon/daemon.go +11 -5
@@ -150,8 +150,14 @@ type Daemon struct {
150150 done chan struct{}
151151 doneOnce sync.Once
152152
153- sessWg sync.WaitGroup // PTY reader goroutines
153+ // sessWg owns attention animation, bar-script polling, CWD sampling,
154+ // snapshot save/restore, and floating-session launch goroutines.
155+ sessWg sync.WaitGroup
154156 connWg sync.WaitGroup // per-connection handler goroutines
157+ // attachmentCleanupWg owns replacement transport closes and retired render
158+ // worker joins. Only connection handlers add work; Serve waits for those
159+ // handlers before joining this group, so no Add races its terminal Wait.
160+ attachmentCleanupWg sync.WaitGroup
155161 }
156162
157163 type parkedAttachment struct {
@@ -434,6 +440,7 @@ func (d *Daemon) Serve(ctx context.Context, l ports.Listener) error {
434440 // entered killSession may still submit its terminal capture after the
435441 // registry snapshot has been removed.
436442 d.connWg.Wait()
443+ d.attachmentCleanupWg.Wait()
437444 d.stopSnapshotEncodeWorker()
438445 d.shutdownAll(ports.ReasonServerShutdown)
439446 d.sessWg.Wait()
@@ -660,8 +667,8 @@ type protoErr struct {
660667 func (e *protoErr) Error() string { return e.text }
661668
662669 // finishAttach completes an attachment prepared while d.mu is held. It
663-// publishes the terminal state before releasing d.mu, then performs worker
664-// joins and replacement teardown without the daemon lock.
670+// publishes the terminal state before releasing d.mu, then queues replacement
671+// teardown so obsolete worker joins never delay the new handshake.
665672 func (d *Daemon) finishAttach(sess *session, tr ports.Transport, sz domain.Size, term terminalEnv, h ports.Hello) *attachedClient {
666673 ac, old, cleanup := d.attachClientDeferred(sess, tr, sz, attachClientOptions{
667674 clientID: h.ClientID,
@@ -672,8 +679,7 @@ func (d *Daemon) finishAttach(sess *session, tr ports.Transport, sz domain.Size,
672679 sess.terminal = term
673680 sess.mu.Unlock()
674681 d.mu.Unlock()
675- cleanup.finish()
676- d.detachReplacedClient(old)
682+ d.retireReplacedClient(old, cleanup)
677683 return ac
678684 }
679685
@@ -150,8 +150,14 @@ type Daemon struct {
150 done chan struct{}150 done chan struct{}
151 doneOnce sync.Once151 doneOnce sync.Once
152 152
153- sessWg sync.WaitGroup // PTY reader goroutines153+ // sessWg owns attention animation, bar-script polling, CWD sampling,
154+ // snapshot save/restore, and floating-session launch goroutines.
155+ sessWg sync.WaitGroup
154 connWg sync.WaitGroup // per-connection handler goroutines156 connWg sync.WaitGroup // per-connection handler goroutines
157+ // attachmentCleanupWg owns replacement transport closes and retired render
158+ // worker joins. Only connection handlers add work; Serve waits for those
159+ // handlers before joining this group, so no Add races its terminal Wait.
160+ attachmentCleanupWg sync.WaitGroup
155 }161 }
156 162
157 type parkedAttachment struct {163 type parkedAttachment struct {
@@ -434,6 +440,7 @@ func (d *Daemon) Serve(ctx context.Context, l ports.Listener) error {
434 // entered killSession may still submit its terminal capture after the440 // entered killSession may still submit its terminal capture after the
435 // registry snapshot has been removed.441 // registry snapshot has been removed.
436 d.connWg.Wait()442 d.connWg.Wait()
443+ d.attachmentCleanupWg.Wait()
437 d.stopSnapshotEncodeWorker()444 d.stopSnapshotEncodeWorker()
438 d.shutdownAll(ports.ReasonServerShutdown)445 d.shutdownAll(ports.ReasonServerShutdown)
439 d.sessWg.Wait()446 d.sessWg.Wait()
@@ -660,8 +667,8 @@ type protoErr struct {
660 func (e *protoErr) Error() string { return e.text }667 func (e *protoErr) Error() string { return e.text }
661 668
662 // finishAttach completes an attachment prepared while d.mu is held. It669 // finishAttach completes an attachment prepared while d.mu is held. It
663-// publishes the terminal state before releasing d.mu, then performs worker670+// publishes the terminal state before releasing d.mu, then queues replacement
664-// joins and replacement teardown without the daemon lock.671+// teardown so obsolete worker joins never delay the new handshake.
665 func (d *Daemon) finishAttach(sess *session, tr ports.Transport, sz domain.Size, term terminalEnv, h ports.Hello) *attachedClient {672 func (d *Daemon) finishAttach(sess *session, tr ports.Transport, sz domain.Size, term terminalEnv, h ports.Hello) *attachedClient {
666 ac, old, cleanup := d.attachClientDeferred(sess, tr, sz, attachClientOptions{673 ac, old, cleanup := d.attachClientDeferred(sess, tr, sz, attachClientOptions{
667 clientID: h.ClientID,674 clientID: h.ClientID,
@@ -672,8 +679,7 @@ func (d *Daemon) finishAttach(sess *session, tr ports.Transport, sz domain.Size,
672 sess.terminal = term679 sess.terminal = term
673 sess.mu.Unlock()680 sess.mu.Unlock()
674 d.mu.Unlock()681 d.mu.Unlock()
675- cleanup.finish()682+ d.retireReplacedClient(old, cleanup)
676- d.detachReplacedClient(old)
677 return ac683 return ac
678 }684 }
679 685
M internal/usecase/daemon/daemon_test.go +12 -5
@@ -295,6 +295,7 @@ type welcomeBlockingTransport struct {
295295 welcomeEntered chan struct{}
296296 releaseWelcome chan struct{}
297297 recvDone chan struct{}
298+ releaseOnce sync.Once
298299 closeOnce sync.Once
299300 }
300301
@@ -320,10 +321,16 @@ func newWelcomeBlockingTransport(t *testing.T) *welcomeBlockingTransport {
320321 return ports.Frame{}, io.EOF
321322 }).Maybe()
322323 b.tr.EXPECT().Close().Return(nil).Maybe()
324+ t.Cleanup(b.finish)
323325 return b
324326 }
325327
328+func (b *welcomeBlockingTransport) release() {
329+ b.releaseOnce.Do(func() { close(b.releaseWelcome) })
330+}
331+
326332 func (b *welcomeBlockingTransport) finish() {
333+ b.release()
327334 b.closeOnce.Do(func() { close(b.recvDone) })
328335 }
329336
@@ -448,8 +455,8 @@ func TestHandleHelloDefersFreshOutputUntilWelcome(t *testing.T) {
448455 close(done)
449456 }()
450457
451- <-tr.welcomeEntered
452- welcome := <-tr.sends
458+ awaitTestCompletion(t, tr.welcomeEntered, "timed out waiting for Welcome send")
459+ welcome := awaitFrame(t, tr.sends, ports.MsgWelcome)
453460 require.Equal(t, ports.MsgWelcome, welcome.Type)
454461 sess := firstSession(d)
455462 sess.mu.Lock()
@@ -469,16 +476,16 @@ func TestHandleHelloDefersFreshOutputUntilWelcome(t *testing.T) {
469476 rc.mu.Unlock()
470477 require.NotNil(t, workerDone)
471478 timer.ch <- time.Time{}
472- awaitHandshakeWorker(t, workerDone)
479+ awaitTestCompletion(t, workerDone, "coordinator deadline worker did not complete")
473480 requireNoCoordinatorOutputFrame(t, tr.sends)
474481
475- close(tr.releaseWelcome)
482+ tr.release()
476483 output := awaitFrame(t, tr.sends, ports.MsgOutput)
477484 first, err := ports.UnmarshalOutput(output.Payload)
478485 require.NoError(t, err)
479486 require.Zero(t, first.BaseStateNum, "the first post-Welcome frame is full")
480487 tr.finish()
481- <-done
488+ awaitTestCompletion(t, done, "timed out waiting for Welcome handler completion")
482489 requireNoCoordinatorOutputFrame(t, tr.sends)
483490 }
484491
@@ -295,6 +295,7 @@ type welcomeBlockingTransport struct {
295 welcomeEntered chan struct{}295 welcomeEntered chan struct{}
296 releaseWelcome chan struct{}296 releaseWelcome chan struct{}
297 recvDone chan struct{}297 recvDone chan struct{}
298+ releaseOnce sync.Once
298 closeOnce sync.Once299 closeOnce sync.Once
299 }300 }
300 301
@@ -320,10 +321,16 @@ func newWelcomeBlockingTransport(t *testing.T) *welcomeBlockingTransport {
320 return ports.Frame{}, io.EOF321 return ports.Frame{}, io.EOF
321 }).Maybe()322 }).Maybe()
322 b.tr.EXPECT().Close().Return(nil).Maybe()323 b.tr.EXPECT().Close().Return(nil).Maybe()
324+ t.Cleanup(b.finish)
323 return b325 return b
324 }326 }
325 327
328+func (b *welcomeBlockingTransport) release() {
329+ b.releaseOnce.Do(func() { close(b.releaseWelcome) })
330+}
331+
326 func (b *welcomeBlockingTransport) finish() {332 func (b *welcomeBlockingTransport) finish() {
333+ b.release()
327 b.closeOnce.Do(func() { close(b.recvDone) })334 b.closeOnce.Do(func() { close(b.recvDone) })
328 }335 }
329 336
@@ -448,8 +455,8 @@ func TestHandleHelloDefersFreshOutputUntilWelcome(t *testing.T) {
448 close(done)455 close(done)
449 }()456 }()
450 457
451- <-tr.welcomeEntered458+ awaitTestCompletion(t, tr.welcomeEntered, "timed out waiting for Welcome send")
452- welcome := <-tr.sends459+ welcome := awaitFrame(t, tr.sends, ports.MsgWelcome)
453 require.Equal(t, ports.MsgWelcome, welcome.Type)460 require.Equal(t, ports.MsgWelcome, welcome.Type)
454 sess := firstSession(d)461 sess := firstSession(d)
455 sess.mu.Lock()462 sess.mu.Lock()
@@ -469,16 +476,16 @@ func TestHandleHelloDefersFreshOutputUntilWelcome(t *testing.T) {
469 rc.mu.Unlock()476 rc.mu.Unlock()
470 require.NotNil(t, workerDone)477 require.NotNil(t, workerDone)
471 timer.ch <- time.Time{}478 timer.ch <- time.Time{}
472- awaitHandshakeWorker(t, workerDone)479+ awaitTestCompletion(t, workerDone, "coordinator deadline worker did not complete")
473 requireNoCoordinatorOutputFrame(t, tr.sends)480 requireNoCoordinatorOutputFrame(t, tr.sends)
474 481
475- close(tr.releaseWelcome)482+ tr.release()
476 output := awaitFrame(t, tr.sends, ports.MsgOutput)483 output := awaitFrame(t, tr.sends, ports.MsgOutput)
477 first, err := ports.UnmarshalOutput(output.Payload)484 first, err := ports.UnmarshalOutput(output.Payload)
478 require.NoError(t, err)485 require.NoError(t, err)
479 require.Zero(t, first.BaseStateNum, "the first post-Welcome frame is full")486 require.Zero(t, first.BaseStateNum, "the first post-Welcome frame is full")
480 tr.finish()487 tr.finish()
481- <-done488+ awaitTestCompletion(t, done, "timed out waiting for Welcome handler completion")
482 requireNoCoordinatorOutputFrame(t, tr.sends)489 requireNoCoordinatorOutputFrame(t, tr.sends)
483 }490 }
484 491
M internal/usecase/daemon/input_test.go +12 -12
@@ -58,11 +58,11 @@ func TestAltDigitSwitchesBetweenThreeTabs(t *testing.T) {
5858 return len(sessions.Sessions) == 1 && sessions.Sessions[0].Tabs == 3
5959 }, 2*time.Second, 5*time.Millisecond)
6060 require.Eventually(t, func() bool { return len(writes1) == 1 }, 2*time.Second, 5*time.Millisecond)
61- require.Equal(t, []byte("A"), <-writes1)
61+ requirePTYWrite(t, writes1, []byte("A"))
6262 require.Eventually(t, func() bool { return len(writes2) == 1 }, 2*time.Second, 5*time.Millisecond)
63- require.Equal(t, []byte("B"), <-writes2)
63+ requirePTYWrite(t, writes2, []byte("B"))
6464 require.Eventually(t, func() bool { return len(writes3) == 1 }, 2*time.Second, 5*time.Millisecond)
65- require.Equal(t, []byte("C"), <-writes3)
65+ requirePTYWrite(t, writes3, []byte("C"))
6666
6767 d.mu.Lock()
6868 var sess *session
@@ -98,7 +98,7 @@ func TestSwitchTabFirstFrameDoesNotReuseSamePaneIDCapture(t *testing.T) {
9898 source.screen.Write([]byte("source"))
9999 source.mu.Unlock()
100100 d.paint(sess, ac, true, nil)
101- first := <-sends
101+ first := awaitFrame(t, sends, ports.MsgOutput)
102102 firstOutput, err := ports.UnmarshalOutput(first.Payload)
103103 require.NoError(t, err)
104104 terminal := vt.NewScreen(ac.size.Cols, ac.size.Rows)
@@ -114,7 +114,7 @@ func TestSwitchTabFirstFrameDoesNotReuseSamePaneIDCapture(t *testing.T) {
114114 daemonKeyHandler{d: d, ac: ac}.Action(keys.ActionSwitchTab2)
115115 require.Equal(t, 1, activeTabIndex(sess))
116116
117- second := <-sends
117+ second := awaitFrame(t, sends, ports.MsgOutput)
118118 secondOutput, err := ports.UnmarshalOutput(second.Payload)
119119 require.NoError(t, err)
120120 require.Zero(t, secondOutput.BaseStateNum, "tab switch must emit the complete target frame first")
@@ -166,7 +166,7 @@ func TestAltCForwardsToPTY(t *testing.T) {
166166
167167 d.handleInput(sess, ac, []byte("\x1bc"))
168168
169- require.Equal(t, []byte("\x1bc"), <-writes)
169+ requirePTYWrite(t, writes, []byte("\x1bc"))
170170 releasePTY()
171171 }
172172
@@ -396,7 +396,7 @@ func TestBracketedMultilinePasteForwardsDelimitersAndNewlines(t *testing.T) {
396396 paste := []byte("\x1b[200~first line\nsecond line\n\x1b[201~")
397397 d.handleInput(sess, ac, paste)
398398
399- require.Equal(t, paste, <-writes)
399+ requirePTYWrite(t, writes, paste)
400400 releasePTY()
401401 }
402402
@@ -478,7 +478,7 @@ func TestOverlayInputPrecedence(t *testing.T) {
478478 input: []byte("Z"),
479479 check: func(t *testing.T, ac *attachedClient, writes chan []byte) {
480480 t.Helper()
481- require.Equal(t, []byte("Z"), <-writes)
481+ requirePTYWrite(t, writes, []byte("Z"))
482482 },
483483 },
484484 }
@@ -767,7 +767,7 @@ func TestAltXClosesActiveTabAndSelectsRemaining(t *testing.T) {
767767 require.Equal(t, 1, activeTabIndex(sess), "closing middle tab selects the next remaining tab")
768768 d.handleInput(sess, ac, []byte("Z"))
769769 require.Eventually(t, func() bool { return len(writes) == 1 }, 2*time.Second, 5*time.Millisecond)
770- require.Equal(t, []byte("Z"), <-writes)
770+ requirePTYWrite(t, writes, []byte("Z"))
771771
772772 releasePTY1()
773773 releasePTY2()
@@ -871,8 +871,8 @@ func TestMouseAltScreenWheelMapsToArrows(t *testing.T) {
871871 d.handleInput(sess, ac, []byte("\x1b[<64;1;1M"))
872872 d.handleInput(sess, ac, []byte("\x1b[<65;1;1M"))
873873
874- require.Equal(t, []byte("\x1b[A\x1b[A\x1b[A"), <-writes)
875- require.Equal(t, []byte("\x1b[B\x1b[B\x1b[B"), <-writes)
874+ requirePTYWrite(t, writes, []byte("\x1b[A\x1b[A\x1b[A"))
875+ requirePTYWrite(t, writes, []byte("\x1b[B\x1b[B\x1b[B"))
876876 require.Nil(t, ac.overlays.copyMode)
877877 }
878878
@@ -927,7 +927,7 @@ func TestMouseChildForwardingStatusDropAndPressDrop(t *testing.T) {
927927
928928 sess.tabs[0].focusedPane().screen.Write([]byte("\x1b[?1006h"))
929929 d.handleInput(sess, ac, raw)
930- require.Equal(t, []byte("\x1b[<0;2;2M"), <-writes)
930+ requirePTYWrite(t, writes, []byte("\x1b[<0;2;2M"))
931931
932932 d.handleInput(sess, ac, []byte("\x1b[<0;1;1M"))
933933 select {
@@ -58,11 +58,11 @@ func TestAltDigitSwitchesBetweenThreeTabs(t *testing.T) {
58 return len(sessions.Sessions) == 1 && sessions.Sessions[0].Tabs == 358 return len(sessions.Sessions) == 1 && sessions.Sessions[0].Tabs == 3
59 }, 2*time.Second, 5*time.Millisecond)59 }, 2*time.Second, 5*time.Millisecond)
60 require.Eventually(t, func() bool { return len(writes1) == 1 }, 2*time.Second, 5*time.Millisecond)60 require.Eventually(t, func() bool { return len(writes1) == 1 }, 2*time.Second, 5*time.Millisecond)
61- require.Equal(t, []byte("A"), <-writes1)61+ requirePTYWrite(t, writes1, []byte("A"))
62 require.Eventually(t, func() bool { return len(writes2) == 1 }, 2*time.Second, 5*time.Millisecond)62 require.Eventually(t, func() bool { return len(writes2) == 1 }, 2*time.Second, 5*time.Millisecond)
63- require.Equal(t, []byte("B"), <-writes2)63+ requirePTYWrite(t, writes2, []byte("B"))
64 require.Eventually(t, func() bool { return len(writes3) == 1 }, 2*time.Second, 5*time.Millisecond)64 require.Eventually(t, func() bool { return len(writes3) == 1 }, 2*time.Second, 5*time.Millisecond)
65- require.Equal(t, []byte("C"), <-writes3)65+ requirePTYWrite(t, writes3, []byte("C"))
66 66
67 d.mu.Lock()67 d.mu.Lock()
68 var sess *session68 var sess *session
@@ -98,7 +98,7 @@ func TestSwitchTabFirstFrameDoesNotReuseSamePaneIDCapture(t *testing.T) {
98 source.screen.Write([]byte("source"))98 source.screen.Write([]byte("source"))
99 source.mu.Unlock()99 source.mu.Unlock()
100 d.paint(sess, ac, true, nil)100 d.paint(sess, ac, true, nil)
101- first := <-sends101+ first := awaitFrame(t, sends, ports.MsgOutput)
102 firstOutput, err := ports.UnmarshalOutput(first.Payload)102 firstOutput, err := ports.UnmarshalOutput(first.Payload)
103 require.NoError(t, err)103 require.NoError(t, err)
104 terminal := vt.NewScreen(ac.size.Cols, ac.size.Rows)104 terminal := vt.NewScreen(ac.size.Cols, ac.size.Rows)
@@ -114,7 +114,7 @@ func TestSwitchTabFirstFrameDoesNotReuseSamePaneIDCapture(t *testing.T) {
114 daemonKeyHandler{d: d, ac: ac}.Action(keys.ActionSwitchTab2)114 daemonKeyHandler{d: d, ac: ac}.Action(keys.ActionSwitchTab2)
115 require.Equal(t, 1, activeTabIndex(sess))115 require.Equal(t, 1, activeTabIndex(sess))
116 116
117- second := <-sends117+ second := awaitFrame(t, sends, ports.MsgOutput)
118 secondOutput, err := ports.UnmarshalOutput(second.Payload)118 secondOutput, err := ports.UnmarshalOutput(second.Payload)
119 require.NoError(t, err)119 require.NoError(t, err)
120 require.Zero(t, secondOutput.BaseStateNum, "tab switch must emit the complete target frame first")120 require.Zero(t, secondOutput.BaseStateNum, "tab switch must emit the complete target frame first")
@@ -166,7 +166,7 @@ func TestAltCForwardsToPTY(t *testing.T) {
166 166
167 d.handleInput(sess, ac, []byte("\x1bc"))167 d.handleInput(sess, ac, []byte("\x1bc"))
168 168
169- require.Equal(t, []byte("\x1bc"), <-writes)169+ requirePTYWrite(t, writes, []byte("\x1bc"))
170 releasePTY()170 releasePTY()
171 }171 }
172 172
@@ -396,7 +396,7 @@ func TestBracketedMultilinePasteForwardsDelimitersAndNewlines(t *testing.T) {
396 paste := []byte("\x1b[200~first line\nsecond line\n\x1b[201~")396 paste := []byte("\x1b[200~first line\nsecond line\n\x1b[201~")
397 d.handleInput(sess, ac, paste)397 d.handleInput(sess, ac, paste)
398 398
399- require.Equal(t, paste, <-writes)399+ requirePTYWrite(t, writes, paste)
400 releasePTY()400 releasePTY()
401 }401 }
402 402
@@ -478,7 +478,7 @@ func TestOverlayInputPrecedence(t *testing.T) {
478 input: []byte("Z"),478 input: []byte("Z"),
479 check: func(t *testing.T, ac *attachedClient, writes chan []byte) {479 check: func(t *testing.T, ac *attachedClient, writes chan []byte) {
480 t.Helper()480 t.Helper()
481- require.Equal(t, []byte("Z"), <-writes)481+ requirePTYWrite(t, writes, []byte("Z"))
482 },482 },
483 },483 },
484 }484 }
@@ -767,7 +767,7 @@ func TestAltXClosesActiveTabAndSelectsRemaining(t *testing.T) {
767 require.Equal(t, 1, activeTabIndex(sess), "closing middle tab selects the next remaining tab")767 require.Equal(t, 1, activeTabIndex(sess), "closing middle tab selects the next remaining tab")
768 d.handleInput(sess, ac, []byte("Z"))768 d.handleInput(sess, ac, []byte("Z"))
769 require.Eventually(t, func() bool { return len(writes) == 1 }, 2*time.Second, 5*time.Millisecond)769 require.Eventually(t, func() bool { return len(writes) == 1 }, 2*time.Second, 5*time.Millisecond)
770- require.Equal(t, []byte("Z"), <-writes)770+ requirePTYWrite(t, writes, []byte("Z"))
771 771
772 releasePTY1()772 releasePTY1()
773 releasePTY2()773 releasePTY2()
@@ -871,8 +871,8 @@ func TestMouseAltScreenWheelMapsToArrows(t *testing.T) {
871 d.handleInput(sess, ac, []byte("\x1b[<64;1;1M"))871 d.handleInput(sess, ac, []byte("\x1b[<64;1;1M"))
872 d.handleInput(sess, ac, []byte("\x1b[<65;1;1M"))872 d.handleInput(sess, ac, []byte("\x1b[<65;1;1M"))
873 873
874- require.Equal(t, []byte("\x1b[A\x1b[A\x1b[A"), <-writes)874+ requirePTYWrite(t, writes, []byte("\x1b[A\x1b[A\x1b[A"))
875- require.Equal(t, []byte("\x1b[B\x1b[B\x1b[B"), <-writes)875+ requirePTYWrite(t, writes, []byte("\x1b[B\x1b[B\x1b[B"))
876 require.Nil(t, ac.overlays.copyMode)876 require.Nil(t, ac.overlays.copyMode)
877 }877 }
878 878
@@ -927,7 +927,7 @@ func TestMouseChildForwardingStatusDropAndPressDrop(t *testing.T) {
927 927
928 sess.tabs[0].focusedPane().screen.Write([]byte("\x1b[?1006h"))928 sess.tabs[0].focusedPane().screen.Write([]byte("\x1b[?1006h"))
929 d.handleInput(sess, ac, raw)929 d.handleInput(sess, ac, raw)
930- require.Equal(t, []byte("\x1b[<0;2;2M"), <-writes)930+ requirePTYWrite(t, writes, []byte("\x1b[<0;2;2M"))
931 931
932 d.handleInput(sess, ac, []byte("\x1b[<0;1;1M"))932 d.handleInput(sess, ac, []byte("\x1b[<0;1;1M"))
933 select {933 select {
M internal/usecase/daemon/overlay_runtime.go +7 -6
@@ -13,12 +13,13 @@ import (
1313 type overlayRuntime struct {
1414 ac *attachedClient
1515
16- pickerMu sync.Mutex
17- picker *picker.Model
18- pickerPreview *tab
19- pickerPreviewSession *session
20- pickerPending []byte
21- pickerESC pendingByteTimer
16+ pickerMu sync.Mutex
17+ picker *picker.Model
18+ pickerPreview *tab
19+ pickerPreviewSession *session
20+ pickerPreviewGeneration uint64
21+ pickerPending []byte
22+ pickerESC pendingByteTimer
2223
2324 paletteMu sync.Mutex
2425 palette *palette.Model
@@ -13,12 +13,13 @@ import (
13 type overlayRuntime struct {13 type overlayRuntime struct {
14 ac *attachedClient14 ac *attachedClient
15 15
16- pickerMu sync.Mutex16+ pickerMu sync.Mutex
17- picker *picker.Model17+ picker *picker.Model
18- pickerPreview *tab18+ pickerPreview *tab
19- pickerPreviewSession *session19+ pickerPreviewSession *session
20- pickerPending []byte20+ pickerPreviewGeneration uint64
21- pickerESC pendingByteTimer21+ pickerPending []byte
22+ pickerESC pendingByteTimer
22 23
23 paletteMu sync.Mutex24 paletteMu sync.Mutex
24 palette *palette.Model25 palette *palette.Model
M internal/usecase/daemon/performance_benchmark_test.go +5 -18
@@ -3,7 +3,6 @@ package daemon
33 import (
44 "fmt"
55 "io"
6- "runtime"
76 "sync"
87 "testing"
98 "time"
@@ -76,7 +75,7 @@ func TestPerformanceFixtureCounters(t *testing.T) {
7675 func TestRenderStageHooksCountProductionBoundariesOnFailedSend(t *testing.T) {
7776 d, sess, ac, sends := newManualSessionWithPTYs(t, nil)
7877 d.paint(sess, ac, true, nil)
79- <-sends
78+ _ = awaitFrame(t, sends, ports.MsgOutput)
8079 var captures, compositions, emissions int
8180 ac.renderStages = renderStageHooks{
8281 capture: func() { captures++ },
@@ -728,7 +727,7 @@ func newPerformanceFixture(t testing.TB, config performanceConfig) *performanceF
728727 sess: sess,
729728 ac: ac,
730729 output: output,
731- snaps: &countingSnapshotStore{done: make(chan struct{}, 1)},
730+ snaps: &countingSnapshotStore{},
732731 pty: pty,
733732 clock: clock,
734733 liveWrites: [][]byte{[]byte("\x1b[1;1HA\x1b[2;2HA"), []byte("\x1b[1;1HB\x1b[2;2HB")},
@@ -823,17 +822,9 @@ func (f *performanceFixture) paintLive() {
823822 func (f *performanceFixture) captureSnapshot() {
824823 f.t.Helper()
825824 require.True(f.t, f.d.captureSession(f.sess))
826- <-f.snaps.done
827- for range 4096 {
828- f.sess.snapshotMu.Lock()
829- pending := f.sess.snapshotPending
830- f.sess.snapshotMu.Unlock()
831- if !pending {
832- return
833- }
834- runtime.Gosched()
835- }
836- f.t.Fatal("snapshot worker did not finish performance capture")
825+ // Worker finalization clears snapshotPending and signals snapshotChanged on
826+ // every success and failure path after persistence has returned.
827+ awaitSnapshotIdle(f.t, f.sess)
837828 }
838829
839830 func (f *performanceFixture) enterCopySearch() {
@@ -1024,7 +1015,6 @@ type countingSnapshotStore struct {
10241015 mu sync.Mutex
10251016 countingSnapshotMetrics
10261017 last []byte
1027- done chan struct{}
10281018 }
10291019
10301020 func (s *countingSnapshotStore) Write(_ string, data []byte) error {
@@ -1033,9 +1023,6 @@ func (s *countingSnapshotStore) Write(_ string, data []byte) error {
10331023 s.bytes += uint64(len(data))
10341024 s.last = data
10351025 s.mu.Unlock()
1036- if s.done != nil {
1037- s.done <- struct{}{}
1038- }
10391026 return nil
10401027 }
10411028 func (*countingSnapshotStore) Load() ([]ports.SnapshotBlob, error) { return nil, nil }
@@ -3,7 +3,6 @@ package daemon
3 import (3 import (
4 "fmt"4 "fmt"
5 "io"5 "io"
6- "runtime"
7 "sync"6 "sync"
8 "testing"7 "testing"
9 "time"8 "time"
@@ -76,7 +75,7 @@ func TestPerformanceFixtureCounters(t *testing.T) {
76 func TestRenderStageHooksCountProductionBoundariesOnFailedSend(t *testing.T) {75 func TestRenderStageHooksCountProductionBoundariesOnFailedSend(t *testing.T) {
77 d, sess, ac, sends := newManualSessionWithPTYs(t, nil)76 d, sess, ac, sends := newManualSessionWithPTYs(t, nil)
78 d.paint(sess, ac, true, nil)77 d.paint(sess, ac, true, nil)
79- <-sends78+ _ = awaitFrame(t, sends, ports.MsgOutput)
80 var captures, compositions, emissions int79 var captures, compositions, emissions int
81 ac.renderStages = renderStageHooks{80 ac.renderStages = renderStageHooks{
82 capture: func() { captures++ },81 capture: func() { captures++ },
@@ -728,7 +727,7 @@ func newPerformanceFixture(t testing.TB, config performanceConfig) *performanceF
728 sess: sess,727 sess: sess,
729 ac: ac,728 ac: ac,
730 output: output,729 output: output,
731- snaps: &countingSnapshotStore{done: make(chan struct{}, 1)},730+ snaps: &countingSnapshotStore{},
732 pty: pty,731 pty: pty,
733 clock: clock,732 clock: clock,
734 liveWrites: [][]byte{[]byte("\x1b[1;1HA\x1b[2;2HA"), []byte("\x1b[1;1HB\x1b[2;2HB")},733 liveWrites: [][]byte{[]byte("\x1b[1;1HA\x1b[2;2HA"), []byte("\x1b[1;1HB\x1b[2;2HB")},
@@ -823,17 +822,9 @@ func (f *performanceFixture) paintLive() {
823 func (f *performanceFixture) captureSnapshot() {822 func (f *performanceFixture) captureSnapshot() {
824 f.t.Helper()823 f.t.Helper()
825 require.True(f.t, f.d.captureSession(f.sess))824 require.True(f.t, f.d.captureSession(f.sess))
826- <-f.snaps.done825+ // Worker finalization clears snapshotPending and signals snapshotChanged on
827- for range 4096 {826+ // every success and failure path after persistence has returned.
828- f.sess.snapshotMu.Lock()827+ awaitSnapshotIdle(f.t, f.sess)
829- pending := f.sess.snapshotPending
830- f.sess.snapshotMu.Unlock()
831- if !pending {
832- return
833- }
834- runtime.Gosched()
835- }
836- f.t.Fatal("snapshot worker did not finish performance capture")
837 }828 }
838 829
839 func (f *performanceFixture) enterCopySearch() {830 func (f *performanceFixture) enterCopySearch() {
@@ -1024,7 +1015,6 @@ type countingSnapshotStore struct {
1024 mu sync.Mutex1015 mu sync.Mutex
1025 countingSnapshotMetrics1016 countingSnapshotMetrics
1026 last []byte1017 last []byte
1027- done chan struct{}
1028 }1018 }
1029 1019
1030 func (s *countingSnapshotStore) Write(_ string, data []byte) error {1020 func (s *countingSnapshotStore) Write(_ string, data []byte) error {
@@ -1033,9 +1023,6 @@ func (s *countingSnapshotStore) Write(_ string, data []byte) error {
1033 s.bytes += uint64(len(data))1023 s.bytes += uint64(len(data))
1034 s.last = data1024 s.last = data
1035 s.mu.Unlock()1025 s.mu.Unlock()
1036- if s.done != nil {
1037- s.done <- struct{}{}
1038- }
1039 return nil1026 return nil
1040 }1027 }
1041 func (*countingSnapshotStore) Load() ([]ports.SnapshotBlob, error) { return nil, nil }1028 func (*countingSnapshotStore) Load() ([]ports.SnapshotBlob, error) { return nil, nil }
M internal/usecase/daemon/picker.go +65 -26
@@ -183,9 +183,10 @@ func (d *Daemon) retainPickerESCLocked(ac *attachedClient) {
183183 ac.overlays.pickerESC.timer = nil
184184 ac.overlays.pickerESC.done = nil
185185 ac.overlays.picker = nil
186+ generation := ac.overlays.pickerPreviewGeneration
186187 ac.overlays.pickerMu.Unlock()
187188
188- d.unregisterPreview(ac)
189+ d.clearPreviewGeneration(ac, generation)
189190 if sess := ac.currentSession(); sess != nil {
190191 d.invalidateRender(sess, ac, true, "picker.go")
191192 }
@@ -215,21 +216,22 @@ func isPickerEscapePrefix(data []byte) bool {
215216 }
216217
217218 func (d *Daemon) registerPreviewForSelection(ac *attachedClient) {
219+ if ac == nil || ac.overlays == nil {
220+ return
221+ }
218222 ac.overlays.pickerMu.Lock()
219223 var target picker.Target
220224 var ok bool
221225 if ac.overlays.picker != nil {
222226 target, ok = ac.overlays.picker.Selected()
223227 }
224- previous := ac.overlays.pickerPreviewSession
228+ previous, previousGeneration := ac.overlays.pickerPreviewSession, ac.overlays.pickerPreviewGeneration
229+ ac.overlays.pickerPreviewGeneration++
230+ generation := ac.overlays.pickerPreviewGeneration
225231 ac.overlays.pickerPreviewSession = nil
226232 ac.overlays.pickerPreview = nil
227233 ac.overlays.pickerMu.Unlock()
228- if previous != nil {
229- if rc := previous.renderCoordinator(); rc != nil {
230- rc.teardownPreviewFor(ac)
231- }
232- }
234+ d.teardownPreviewSubscription(ac, previous, previousGeneration)
233235 if !ok {
234236 return
235237 }
@@ -240,13 +242,16 @@ func (d *Daemon) registerPreviewForSelection(ac *attachedClient) {
240242 }
241243 ac.overlays.pickerMu.Lock()
242244 // Selection may have changed while the target was resolved.
243- if ac.overlays.picker == nil {
244- ac.overlays.pickerMu.Unlock()
245- return
245+ selected, stillSelected := ac.overlays.picker.Selected()
246+ valid := ac.overlays.pickerPreviewGeneration == generation && stillSelected && selected == target
247+ if valid {
248+ ac.overlays.pickerPreview = next
249+ ac.overlays.pickerPreviewSession = targetSess
246250 }
247- ac.overlays.pickerPreview = next
248- ac.overlays.pickerPreviewSession = targetSess
249251 ac.overlays.pickerMu.Unlock()
252+ if !valid {
253+ return
254+ }
250255 // A same-session preview is already invalidated by its own coordinator.
251256 // Cross-session previews subscribe to the target's producer wake instead
252257 // of starting a direct paint/timer path.
@@ -254,16 +259,38 @@ func (d *Daemon) registerPreviewForSelection(ac *attachedClient) {
254259 return
255260 }
256261 rc := d.attachCoordinator(targetSess, nil, nil, false)
257- rc.subscribePreviewFor(ac, func(renderWake) {
258- ac.overlays.pickerMu.Lock()
259- valid := ac.overlays.pickerPreviewSession == targetSess
260- ac.overlays.pickerMu.Unlock()
261- if valid {
262+ rc.subscribePreviewFor(ac, generation, func(renderWake) {
263+ if pickerPreviewCurrent(ac, targetSess, next, generation) {
262264 if owner := ac.currentSession(); owner != nil {
263265 d.invalidateRender(owner, ac, false, "picker preview")
264266 }
265267 }
266268 })
269+ // subscribePreviewFor is deliberately outside pickerMu. Revalidate after it
270+ // returns and remove only this generation if selection changed meanwhile.
271+ d.revalidatePreviewSubscription(ac, rc, targetSess, next, generation)
272+}
273+
274+func pickerPreviewCurrent(ac *attachedClient, targetSess *session, next *tab, generation uint64) bool {
275+ ac.overlays.pickerMu.Lock()
276+ defer ac.overlays.pickerMu.Unlock()
277+ return ac.overlays.pickerPreviewGeneration == generation && ac.overlays.pickerPreviewSession == targetSess && ac.overlays.pickerPreview == next
278+}
279+
280+// revalidatePreviewSubscription removes a subscription that lost the picker
281+// selection race after it was installed.
282+func (d *Daemon) revalidatePreviewSubscription(ac *attachedClient, rc *renderCoordinator, targetSess *session, next *tab, generation uint64) {
283+ if !pickerPreviewCurrent(ac, targetSess, next, generation) {
284+ rc.teardownPreviewFor(ac, generation)
285+ }
286+}
287+
288+func (d *Daemon) teardownPreviewSubscription(ac *attachedClient, previous *session, generation uint64) {
289+ if previous != nil {
290+ if rc := previous.renderCoordinator(); rc != nil {
291+ rc.teardownPreviewFor(ac, generation)
292+ }
293+ }
267294 }
268295
269296 func (d *Daemon) unregisterPreview(ac *attachedClient) {
@@ -271,15 +298,26 @@ func (d *Daemon) unregisterPreview(ac *attachedClient) {
271298 return
272299 }
273300 ac.overlays.pickerMu.Lock()
301+ generation := ac.overlays.pickerPreviewGeneration
302+ ac.overlays.pickerMu.Unlock()
303+ d.clearPreviewGeneration(ac, generation)
304+}
305+
306+// clearPreviewGeneration removes the preview only if it is still the observed
307+// selection. Teardown paths may race a new picker selection.
308+func (d *Daemon) clearPreviewGeneration(ac *attachedClient, generation uint64) bool {
309+ ac.overlays.pickerMu.Lock()
310+ if ac.overlays.pickerPreviewGeneration != generation {
311+ ac.overlays.pickerMu.Unlock()
312+ return false
313+ }
274314 previous := ac.overlays.pickerPreviewSession
315+ ac.overlays.pickerPreviewGeneration++
275316 ac.overlays.pickerPreviewSession = nil
276317 ac.overlays.pickerPreview = nil
277318 ac.overlays.pickerMu.Unlock()
278- if previous != nil {
279- if rc := previous.renderCoordinator(); rc != nil {
280- rc.teardownPreviewFor(ac)
281- }
282- }
319+ d.teardownPreviewSubscription(ac, previous, generation)
320+ return true
283321 }
284322
285323 // clearDestroyedTabPreview clears coordinator-owned picker state for clients
@@ -303,10 +341,10 @@ func (d *Daemon) clearDestroyedTabPreview(tb *tab) {
303341 continue
304342 }
305343 ac.overlays.pickerMu.Lock()
306- cleared := ac.overlays.pickerPreview == tb
344+ generation := ac.overlays.pickerPreviewGeneration
345+ observed := ac.overlays.pickerPreview == tb
307346 ac.overlays.pickerMu.Unlock()
308- if cleared {
309- d.unregisterPreview(ac)
347+ if observed && d.clearPreviewGeneration(ac, generation) {
310348 if sess := ac.currentSession(); sess != nil {
311349 d.invalidateRender(sess, ac, true, "picker.go")
312350 }
@@ -319,8 +357,9 @@ func (d *Daemon) closePicker(ac *attachedClient) {
319357 ac.overlays.picker = nil
320358 ac.overlays.pickerPending = nil
321359 d.stopPickerPendingTimerLocked(ac)
360+ generation := ac.overlays.pickerPreviewGeneration
322361 ac.overlays.pickerMu.Unlock()
323- d.unregisterPreview(ac)
362+ d.clearPreviewGeneration(ac, generation)
324363 }
325364
326365 func (d *Daemon) tabByTarget(target picker.Target) *tab {
@@ -183,9 +183,10 @@ func (d *Daemon) retainPickerESCLocked(ac *attachedClient) {
183 ac.overlays.pickerESC.timer = nil183 ac.overlays.pickerESC.timer = nil
184 ac.overlays.pickerESC.done = nil184 ac.overlays.pickerESC.done = nil
185 ac.overlays.picker = nil185 ac.overlays.picker = nil
186+ generation := ac.overlays.pickerPreviewGeneration
186 ac.overlays.pickerMu.Unlock()187 ac.overlays.pickerMu.Unlock()
187 188
188- d.unregisterPreview(ac)189+ d.clearPreviewGeneration(ac, generation)
189 if sess := ac.currentSession(); sess != nil {190 if sess := ac.currentSession(); sess != nil {
190 d.invalidateRender(sess, ac, true, "picker.go")191 d.invalidateRender(sess, ac, true, "picker.go")
191 }192 }
@@ -215,21 +216,22 @@ func isPickerEscapePrefix(data []byte) bool {
215 }216 }
216 217
217 func (d *Daemon) registerPreviewForSelection(ac *attachedClient) {218 func (d *Daemon) registerPreviewForSelection(ac *attachedClient) {
219+ if ac == nil || ac.overlays == nil {
220+ return
221+ }
218 ac.overlays.pickerMu.Lock()222 ac.overlays.pickerMu.Lock()
219 var target picker.Target223 var target picker.Target
220 var ok bool224 var ok bool
221 if ac.overlays.picker != nil {225 if ac.overlays.picker != nil {
222 target, ok = ac.overlays.picker.Selected()226 target, ok = ac.overlays.picker.Selected()
223 }227 }
224- previous := ac.overlays.pickerPreviewSession228+ previous, previousGeneration := ac.overlays.pickerPreviewSession, ac.overlays.pickerPreviewGeneration
229+ ac.overlays.pickerPreviewGeneration++
230+ generation := ac.overlays.pickerPreviewGeneration
225 ac.overlays.pickerPreviewSession = nil231 ac.overlays.pickerPreviewSession = nil
226 ac.overlays.pickerPreview = nil232 ac.overlays.pickerPreview = nil
227 ac.overlays.pickerMu.Unlock()233 ac.overlays.pickerMu.Unlock()
228- if previous != nil {234+ d.teardownPreviewSubscription(ac, previous, previousGeneration)
229- if rc := previous.renderCoordinator(); rc != nil {
230- rc.teardownPreviewFor(ac)
231- }
232- }
233 if !ok {235 if !ok {
234 return236 return
235 }237 }
@@ -240,13 +242,16 @@ func (d *Daemon) registerPreviewForSelection(ac *attachedClient) {
240 }242 }
241 ac.overlays.pickerMu.Lock()243 ac.overlays.pickerMu.Lock()
242 // Selection may have changed while the target was resolved.244 // Selection may have changed while the target was resolved.
243- if ac.overlays.picker == nil {245+ selected, stillSelected := ac.overlays.picker.Selected()
244- ac.overlays.pickerMu.Unlock()246+ valid := ac.overlays.pickerPreviewGeneration == generation && stillSelected && selected == target
245- return247+ if valid {
248+ ac.overlays.pickerPreview = next
249+ ac.overlays.pickerPreviewSession = targetSess
246 }250 }
247- ac.overlays.pickerPreview = next
248- ac.overlays.pickerPreviewSession = targetSess
249 ac.overlays.pickerMu.Unlock()251 ac.overlays.pickerMu.Unlock()
252+ if !valid {
253+ return
254+ }
250 // A same-session preview is already invalidated by its own coordinator.255 // A same-session preview is already invalidated by its own coordinator.
251 // Cross-session previews subscribe to the target's producer wake instead256 // Cross-session previews subscribe to the target's producer wake instead
252 // of starting a direct paint/timer path.257 // of starting a direct paint/timer path.
@@ -254,16 +259,38 @@ func (d *Daemon) registerPreviewForSelection(ac *attachedClient) {
254 return259 return
255 }260 }
256 rc := d.attachCoordinator(targetSess, nil, nil, false)261 rc := d.attachCoordinator(targetSess, nil, nil, false)
257- rc.subscribePreviewFor(ac, func(renderWake) {262+ rc.subscribePreviewFor(ac, generation, func(renderWake) {
258- ac.overlays.pickerMu.Lock()263+ if pickerPreviewCurrent(ac, targetSess, next, generation) {
259- valid := ac.overlays.pickerPreviewSession == targetSess
260- ac.overlays.pickerMu.Unlock()
261- if valid {
262 if owner := ac.currentSession(); owner != nil {264 if owner := ac.currentSession(); owner != nil {
263 d.invalidateRender(owner, ac, false, "picker preview")265 d.invalidateRender(owner, ac, false, "picker preview")
264 }266 }
265 }267 }
266 })268 })
269+ // subscribePreviewFor is deliberately outside pickerMu. Revalidate after it
270+ // returns and remove only this generation if selection changed meanwhile.
271+ d.revalidatePreviewSubscription(ac, rc, targetSess, next, generation)
272+}
273+
274+func pickerPreviewCurrent(ac *attachedClient, targetSess *session, next *tab, generation uint64) bool {
275+ ac.overlays.pickerMu.Lock()
276+ defer ac.overlays.pickerMu.Unlock()
277+ return ac.overlays.pickerPreviewGeneration == generation && ac.overlays.pickerPreviewSession == targetSess && ac.overlays.pickerPreview == next
278+}
279+
280+// revalidatePreviewSubscription removes a subscription that lost the picker
281+// selection race after it was installed.
282+func (d *Daemon) revalidatePreviewSubscription(ac *attachedClient, rc *renderCoordinator, targetSess *session, next *tab, generation uint64) {
283+ if !pickerPreviewCurrent(ac, targetSess, next, generation) {
284+ rc.teardownPreviewFor(ac, generation)
285+ }
286+}
287+
288+func (d *Daemon) teardownPreviewSubscription(ac *attachedClient, previous *session, generation uint64) {
289+ if previous != nil {
290+ if rc := previous.renderCoordinator(); rc != nil {
291+ rc.teardownPreviewFor(ac, generation)
292+ }
293+ }
267 }294 }
268 295
269 func (d *Daemon) unregisterPreview(ac *attachedClient) {296 func (d *Daemon) unregisterPreview(ac *attachedClient) {
@@ -271,15 +298,26 @@ func (d *Daemon) unregisterPreview(ac *attachedClient) {
271 return298 return
272 }299 }
273 ac.overlays.pickerMu.Lock()300 ac.overlays.pickerMu.Lock()
301+ generation := ac.overlays.pickerPreviewGeneration
302+ ac.overlays.pickerMu.Unlock()
303+ d.clearPreviewGeneration(ac, generation)
304+}
305+
306+// clearPreviewGeneration removes the preview only if it is still the observed
307+// selection. Teardown paths may race a new picker selection.
308+func (d *Daemon) clearPreviewGeneration(ac *attachedClient, generation uint64) bool {
309+ ac.overlays.pickerMu.Lock()
310+ if ac.overlays.pickerPreviewGeneration != generation {
311+ ac.overlays.pickerMu.Unlock()
312+ return false
313+ }
274 previous := ac.overlays.pickerPreviewSession314 previous := ac.overlays.pickerPreviewSession
315+ ac.overlays.pickerPreviewGeneration++
275 ac.overlays.pickerPreviewSession = nil316 ac.overlays.pickerPreviewSession = nil
276 ac.overlays.pickerPreview = nil317 ac.overlays.pickerPreview = nil
277 ac.overlays.pickerMu.Unlock()318 ac.overlays.pickerMu.Unlock()
278- if previous != nil {319+ d.teardownPreviewSubscription(ac, previous, generation)
279- if rc := previous.renderCoordinator(); rc != nil {320+ return true
280- rc.teardownPreviewFor(ac)
281- }
282- }
283 }321 }
284 322
285 // clearDestroyedTabPreview clears coordinator-owned picker state for clients323 // clearDestroyedTabPreview clears coordinator-owned picker state for clients
@@ -303,10 +341,10 @@ func (d *Daemon) clearDestroyedTabPreview(tb *tab) {
303 continue341 continue
304 }342 }
305 ac.overlays.pickerMu.Lock()343 ac.overlays.pickerMu.Lock()
306- cleared := ac.overlays.pickerPreview == tb344+ generation := ac.overlays.pickerPreviewGeneration
345+ observed := ac.overlays.pickerPreview == tb
307 ac.overlays.pickerMu.Unlock()346 ac.overlays.pickerMu.Unlock()
308- if cleared {347+ if observed && d.clearPreviewGeneration(ac, generation) {
309- d.unregisterPreview(ac)
310 if sess := ac.currentSession(); sess != nil {348 if sess := ac.currentSession(); sess != nil {
311 d.invalidateRender(sess, ac, true, "picker.go")349 d.invalidateRender(sess, ac, true, "picker.go")
312 }350 }
@@ -319,8 +357,9 @@ func (d *Daemon) closePicker(ac *attachedClient) {
319 ac.overlays.picker = nil357 ac.overlays.picker = nil
320 ac.overlays.pickerPending = nil358 ac.overlays.pickerPending = nil
321 d.stopPickerPendingTimerLocked(ac)359 d.stopPickerPendingTimerLocked(ac)
360+ generation := ac.overlays.pickerPreviewGeneration
322 ac.overlays.pickerMu.Unlock()361 ac.overlays.pickerMu.Unlock()
323- d.unregisterPreview(ac)362+ d.clearPreviewGeneration(ac, generation)
324 }363 }
325 364
326 func (d *Daemon) tabByTarget(target picker.Target) *tab {365 func (d *Daemon) tabByTarget(target picker.Target) *tab {
M internal/usecase/daemon/picker_test.go +66 -0
@@ -477,6 +477,72 @@ func TestPickerCrossSessionSwitchCopiesTerminalEnvForFutureTabs(t *testing.T) {
477477 require.NotContains(t, openedEnv, "TERM=xterm-direct")
478478 }
479479
480+func TestPickerPreviewGenerationRejectsStaleSubscriptionReplacement(t *testing.T) {
481+ rc := newRenderCoordinator(renderCoordinatorOptions{})
482+ viewer := &attachedClient{}
483+ newer := func(renderWake) {}
484+
485+ require.True(t, rc.subscribePreviewFor(viewer, 2, newer))
486+ require.False(t, rc.subscribePreviewFor(viewer, 1, func(renderWake) {}))
487+ rc.teardownPreviewFor(viewer, 1)
488+
489+ rc.mu.Lock()
490+ subscription, ok := rc.previewWakes[viewer]
491+ rc.mu.Unlock()
492+ require.True(t, ok)
493+ require.Equal(t, uint64(2), subscription.generation)
494+ require.NotNil(t, subscription.fn)
495+}
496+
497+func TestPickerPreviewPostSubscribeRevalidationKeepsNewerSubscription(t *testing.T) {
498+ d := newTestDaemon(t, nil, stubClock{})
499+ viewer := &attachedClient{}
500+ viewer.initOverlays()
501+ target := &session{}
502+ tab := &tab{}
503+ rc := newRenderCoordinator(renderCoordinatorOptions{})
504+
505+ viewer.overlays.pickerMu.Lock()
506+ viewer.overlays.pickerPreviewGeneration = 2
507+ viewer.overlays.pickerPreviewSession = target
508+ viewer.overlays.pickerPreview = tab
509+ viewer.overlays.pickerMu.Unlock()
510+ require.True(t, rc.subscribePreviewFor(viewer, 2, func(renderWake) {}))
511+
512+ // This is the exact post-subscribe check from registerPreviewForSelection.
513+ // It represents a generation-1 subscriber resuming after generation 2 won.
514+ d.revalidatePreviewSubscription(viewer, rc, target, tab, 1)
515+
516+ rc.mu.Lock()
517+ subscription, ok := rc.previewWakes[viewer]
518+ rc.mu.Unlock()
519+ require.True(t, ok)
520+ require.Equal(t, uint64(2), subscription.generation)
521+}
522+
523+func TestPickerDestroyedTabCleanupDoesNotClearNewerGeneration(t *testing.T) {
524+ d := newTestDaemon(t, nil, stubClock{})
525+ viewer := &attachedClient{}
526+ viewer.initOverlays()
527+ newerSession := &session{}
528+ newerTab := &tab{}
529+ rc := newRenderCoordinator(renderCoordinatorOptions{})
530+
531+ viewer.overlays.pickerMu.Lock()
532+ viewer.overlays.pickerPreviewGeneration = 2
533+ viewer.overlays.pickerPreviewSession = newerSession
534+ viewer.overlays.pickerPreview = newerTab
535+ viewer.overlays.pickerMu.Unlock()
536+ require.True(t, rc.subscribePreviewFor(viewer, 2, func(renderWake) {}))
537+
538+ require.False(t, d.clearPreviewGeneration(viewer, 1))
539+ require.True(t, pickerPreviewCurrent(viewer, newerSession, newerTab, 2))
540+ rc.mu.Lock()
541+ _, subscribed := rc.previewWakes[viewer]
542+ rc.mu.Unlock()
543+ require.True(t, subscribed)
544+}
545+
480546 func TestPickerPreviewSinglePaneSnapshotsFocusedPane(t *testing.T) {
481547 tb := newTab(nil, domain.Size{Cols: 10, Rows: 3})
482548 p := tb.focusedPane()
@@ -477,6 +477,72 @@ func TestPickerCrossSessionSwitchCopiesTerminalEnvForFutureTabs(t *testing.T) {
477 require.NotContains(t, openedEnv, "TERM=xterm-direct")477 require.NotContains(t, openedEnv, "TERM=xterm-direct")
478 }478 }
479 479
480+func TestPickerPreviewGenerationRejectsStaleSubscriptionReplacement(t *testing.T) {
481+ rc := newRenderCoordinator(renderCoordinatorOptions{})
482+ viewer := &attachedClient{}
483+ newer := func(renderWake) {}
484+
485+ require.True(t, rc.subscribePreviewFor(viewer, 2, newer))
486+ require.False(t, rc.subscribePreviewFor(viewer, 1, func(renderWake) {}))
487+ rc.teardownPreviewFor(viewer, 1)
488+
489+ rc.mu.Lock()
490+ subscription, ok := rc.previewWakes[viewer]
491+ rc.mu.Unlock()
492+ require.True(t, ok)
493+ require.Equal(t, uint64(2), subscription.generation)
494+ require.NotNil(t, subscription.fn)
495+}
496+
497+func TestPickerPreviewPostSubscribeRevalidationKeepsNewerSubscription(t *testing.T) {
498+ d := newTestDaemon(t, nil, stubClock{})
499+ viewer := &attachedClient{}
500+ viewer.initOverlays()
501+ target := &session{}
502+ tab := &tab{}
503+ rc := newRenderCoordinator(renderCoordinatorOptions{})
504+
505+ viewer.overlays.pickerMu.Lock()
506+ viewer.overlays.pickerPreviewGeneration = 2
507+ viewer.overlays.pickerPreviewSession = target
508+ viewer.overlays.pickerPreview = tab
509+ viewer.overlays.pickerMu.Unlock()
510+ require.True(t, rc.subscribePreviewFor(viewer, 2, func(renderWake) {}))
511+
512+ // This is the exact post-subscribe check from registerPreviewForSelection.
513+ // It represents a generation-1 subscriber resuming after generation 2 won.
514+ d.revalidatePreviewSubscription(viewer, rc, target, tab, 1)
515+
516+ rc.mu.Lock()
517+ subscription, ok := rc.previewWakes[viewer]
518+ rc.mu.Unlock()
519+ require.True(t, ok)
520+ require.Equal(t, uint64(2), subscription.generation)
521+}
522+
523+func TestPickerDestroyedTabCleanupDoesNotClearNewerGeneration(t *testing.T) {
524+ d := newTestDaemon(t, nil, stubClock{})
525+ viewer := &attachedClient{}
526+ viewer.initOverlays()
527+ newerSession := &session{}
528+ newerTab := &tab{}
529+ rc := newRenderCoordinator(renderCoordinatorOptions{})
530+
531+ viewer.overlays.pickerMu.Lock()
532+ viewer.overlays.pickerPreviewGeneration = 2
533+ viewer.overlays.pickerPreviewSession = newerSession
534+ viewer.overlays.pickerPreview = newerTab
535+ viewer.overlays.pickerMu.Unlock()
536+ require.True(t, rc.subscribePreviewFor(viewer, 2, func(renderWake) {}))
537+
538+ require.False(t, d.clearPreviewGeneration(viewer, 1))
539+ require.True(t, pickerPreviewCurrent(viewer, newerSession, newerTab, 2))
540+ rc.mu.Lock()
541+ _, subscribed := rc.previewWakes[viewer]
542+ rc.mu.Unlock()
543+ require.True(t, subscribed)
544+}
545+
480 func TestPickerPreviewSinglePaneSnapshotsFocusedPane(t *testing.T) {546 func TestPickerPreviewSinglePaneSnapshotsFocusedPane(t *testing.T) {
481 tb := newTab(nil, domain.Size{Cols: 10, Rows: 3})547 tb := newTab(nil, domain.Size{Cols: 10, Rows: 3})
482 p := tb.focusedPane()548 p := tb.focusedPane()
M internal/usecase/daemon/render.go +6 -4
@@ -25,19 +25,21 @@ func (d *Daemon) paneRenderable(sess *session, tb *tab, p *pane) bool {
2525 if sess == nil || tb == nil || p == nil {
2626 return false
2727 }
28- preview := d.tabIsPickerPreview(tb)
2928 sess.mu.Lock()
3029 active := sess.active >= 0 && sess.active < len(sess.tabs) && sess.tabs[sess.active] == tb
3130 attached := sess.client != nil
3231 sess.mu.Unlock()
32+
33+ // The normal attached render path needs no cross-session picker lookup.
34+ // Only inactive or headless tabs can be renderable as a picker preview.
35+ if (!active || !attached) && !d.tabIsPickerPreview(tb) {
36+ return false
37+ }
3338 tb.mu.Lock()
3439 defer tb.mu.Unlock()
3540 if tb.floating.pane == p {
3641 return active && attached && tb.floating.state == floatingVisible
3742 }
38- if (!active || !attached) && !preview {
39- return false
40- }
4143 placements, ok := solvedPlacementsLocked(tb)
4244 if !ok {
4345 return false
@@ -25,19 +25,21 @@ func (d *Daemon) paneRenderable(sess *session, tb *tab, p *pane) bool {
25 if sess == nil || tb == nil || p == nil {25 if sess == nil || tb == nil || p == nil {
26 return false26 return false
27 }27 }
28- preview := d.tabIsPickerPreview(tb)
29 sess.mu.Lock()28 sess.mu.Lock()
30 active := sess.active >= 0 && sess.active < len(sess.tabs) && sess.tabs[sess.active] == tb29 active := sess.active >= 0 && sess.active < len(sess.tabs) && sess.tabs[sess.active] == tb
31 attached := sess.client != nil30 attached := sess.client != nil
32 sess.mu.Unlock()31 sess.mu.Unlock()
32+
33+ // The normal attached render path needs no cross-session picker lookup.
34+ // Only inactive or headless tabs can be renderable as a picker preview.
35+ if (!active || !attached) && !d.tabIsPickerPreview(tb) {
36+ return false
37+ }
33 tb.mu.Lock()38 tb.mu.Lock()
34 defer tb.mu.Unlock()39 defer tb.mu.Unlock()
35 if tb.floating.pane == p {40 if tb.floating.pane == p {
36 return active && attached && tb.floating.state == floatingVisible41 return active && attached && tb.floating.state == floatingVisible
37 }42 }
38- if (!active || !attached) && !preview {
39- return false
40- }
41 placements, ok := solvedPlacementsLocked(tb)43 placements, ok := solvedPlacementsLocked(tb)
42 if !ok {44 if !ok {
43 return false45 return false
M internal/usecase/daemon/render_coordinator.go +35 -17
@@ -44,7 +44,7 @@ type renderCoordinator struct {
4444 // subscriber. previewWakes tracks picker subscriptions by viewer, so one
4545 // inactive session cannot replace another viewer's live preview.
4646 previewWake func(renderWake)
47- previewWakes map[*attachedClient]func(renderWake)
47+ previewWakes map[*attachedClient]previewSubscription
4848
4949 // lease is the sole primary attachment lifecycle state. A revoked lease is
5050 // retained after detach so headless preview delivery remains distinguishable
@@ -457,26 +457,39 @@ func (c *renderCoordinator) subscribePreview(fn func(renderWake)) {
457457 // teardownPreview removes the legacy preview subscription.
458458 func (c *renderCoordinator) teardownPreview() { c.mu.Lock(); c.previewWake = nil; c.mu.Unlock() }
459459
460+type previewSubscription struct {
461+ generation uint64
462+ fn func(renderWake)
463+}
464+
460465 // subscribePreviewFor installs the dynamic picker observer owned by viewer.
461-// The owner key makes target changes, detach, and session teardown precise.
462-func (c *renderCoordinator) subscribePreviewFor(viewer *attachedClient, fn func(renderWake)) {
466+// A newer picker generation wins, so a delayed subscription cannot replace it.
467+func (c *renderCoordinator) subscribePreviewFor(viewer *attachedClient, generation uint64, fn func(renderWake)) bool {
463468 if viewer == nil || fn == nil {
464- return
469+ return false
465470 }
466471 c.mu.Lock()
467- if !c.torndown {
468- if c.previewWakes == nil {
469- c.previewWakes = make(map[*attachedClient]func(renderWake))
470- }
471- c.previewWakes[viewer] = fn
472+ defer c.mu.Unlock()
473+ if c.torndown {
474+ return false
472475 }
473- c.mu.Unlock()
476+ if current, ok := c.previewWakes[viewer]; ok && current.generation > generation {
477+ return false
478+ }
479+ if c.previewWakes == nil {
480+ c.previewWakes = make(map[*attachedClient]previewSubscription)
481+ }
482+ c.previewWakes[viewer] = previewSubscription{generation: generation, fn: fn}
483+ return true
474484 }
475485
476-// teardownPreviewFor removes only viewer's dynamic picker observer.
477-func (c *renderCoordinator) teardownPreviewFor(viewer *attachedClient) {
486+// teardownPreviewFor removes viewer's observer only when it still belongs to
487+// generation. A delayed teardown cannot clear a replacement subscription.
488+func (c *renderCoordinator) teardownPreviewFor(viewer *attachedClient, generation uint64) {
478489 c.mu.Lock()
479- delete(c.previewWakes, viewer)
490+ if current, ok := c.previewWakes[viewer]; ok && current.generation == generation {
491+ delete(c.previewWakes, viewer)
492+ }
480493 c.mu.Unlock()
481494 }
482495
@@ -758,11 +771,16 @@ func (c *renderCoordinator) fireWithTimerWorker(gen uint64, watchdog, deadline b
758771 c.mu.Unlock()
759772 return
760773 }
761- w := renderWake{reset: c.pendingReset, urgent: c.pendingUrgent, coalesced: c.coalesced, watchdog: watchdog, lease: c.lease}
762774 // A detached target has no primary transport or ACK window. Its pending
763775 // state belongs exclusively to picker previews and must be consumed after
764- // notification rather than accumulating ackDeferred forever.
776+ // notification rather than accumulating ackDeferred forever. Do not pass
777+ // its revoked primary lease to preview consumers.
765778 headlessPreviewOnly := c.primaryDetachedLocked()
779+ lease := c.lease
780+ if headlessPreviewOnly {
781+ lease = nil
782+ }
783+ w := renderWake{reset: c.pendingReset, urgent: c.pendingUrgent, coalesced: c.coalesced, watchdog: watchdog, lease: lease}
766784 if !headlessPreviewOnly && (!ready || (c.lease != nil && c.lease.active && !c.lease.ready)) {
767785 var ackStart *ackBlockedSpan
768786 if !ready && deadline {
@@ -860,8 +878,8 @@ func (c *renderCoordinator) takePendingPreviewsLocked() (func(renderWake), []fun
860878 }
861879 c.pendingPreview = false
862880 previews := make([]func(renderWake), 0, len(c.previewWakes))
863- for _, fn := range c.previewWakes {
864- previews = append(previews, fn)
881+ for _, subscription := range c.previewWakes {
882+ previews = append(previews, subscription.fn)
865883 }
866884 return c.previewWake, previews
867885 }
@@ -44,7 +44,7 @@ type renderCoordinator struct {
44 // subscriber. previewWakes tracks picker subscriptions by viewer, so one44 // subscriber. previewWakes tracks picker subscriptions by viewer, so one
45 // inactive session cannot replace another viewer's live preview.45 // inactive session cannot replace another viewer's live preview.
46 previewWake func(renderWake)46 previewWake func(renderWake)
47- previewWakes map[*attachedClient]func(renderWake)47+ previewWakes map[*attachedClient]previewSubscription
48 48
49 // lease is the sole primary attachment lifecycle state. A revoked lease is49 // lease is the sole primary attachment lifecycle state. A revoked lease is
50 // retained after detach so headless preview delivery remains distinguishable50 // retained after detach so headless preview delivery remains distinguishable
@@ -457,26 +457,39 @@ func (c *renderCoordinator) subscribePreview(fn func(renderWake)) {
457 // teardownPreview removes the legacy preview subscription.457 // teardownPreview removes the legacy preview subscription.
458 func (c *renderCoordinator) teardownPreview() { c.mu.Lock(); c.previewWake = nil; c.mu.Unlock() }458 func (c *renderCoordinator) teardownPreview() { c.mu.Lock(); c.previewWake = nil; c.mu.Unlock() }
459 459
460+type previewSubscription struct {
461+ generation uint64
462+ fn func(renderWake)
463+}
464+
460 // subscribePreviewFor installs the dynamic picker observer owned by viewer.465 // subscribePreviewFor installs the dynamic picker observer owned by viewer.
461-// The owner key makes target changes, detach, and session teardown precise.466+// A newer picker generation wins, so a delayed subscription cannot replace it.
462-func (c *renderCoordinator) subscribePreviewFor(viewer *attachedClient, fn func(renderWake)) {467+func (c *renderCoordinator) subscribePreviewFor(viewer *attachedClient, generation uint64, fn func(renderWake)) bool {
463 if viewer == nil || fn == nil {468 if viewer == nil || fn == nil {
464- return469+ return false
465 }470 }
466 c.mu.Lock()471 c.mu.Lock()
467- if !c.torndown {472+ defer c.mu.Unlock()
468- if c.previewWakes == nil {473+ if c.torndown {
469- c.previewWakes = make(map[*attachedClient]func(renderWake))474+ return false
470- }
471- c.previewWakes[viewer] = fn
472 }475 }
473- c.mu.Unlock()476+ if current, ok := c.previewWakes[viewer]; ok && current.generation > generation {
477+ return false
478+ }
479+ if c.previewWakes == nil {
480+ c.previewWakes = make(map[*attachedClient]previewSubscription)
481+ }
482+ c.previewWakes[viewer] = previewSubscription{generation: generation, fn: fn}
483+ return true
474 }484 }
475 485
476-// teardownPreviewFor removes only viewer's dynamic picker observer.486+// teardownPreviewFor removes viewer's observer only when it still belongs to
477-func (c *renderCoordinator) teardownPreviewFor(viewer *attachedClient) {487+// generation. A delayed teardown cannot clear a replacement subscription.
488+func (c *renderCoordinator) teardownPreviewFor(viewer *attachedClient, generation uint64) {
478 c.mu.Lock()489 c.mu.Lock()
479- delete(c.previewWakes, viewer)490+ if current, ok := c.previewWakes[viewer]; ok && current.generation == generation {
491+ delete(c.previewWakes, viewer)
492+ }
480 c.mu.Unlock()493 c.mu.Unlock()
481 }494 }
482 495
@@ -758,11 +771,16 @@ func (c *renderCoordinator) fireWithTimerWorker(gen uint64, watchdog, deadline b
758 c.mu.Unlock()771 c.mu.Unlock()
759 return772 return
760 }773 }
761- w := renderWake{reset: c.pendingReset, urgent: c.pendingUrgent, coalesced: c.coalesced, watchdog: watchdog, lease: c.lease}
762 // A detached target has no primary transport or ACK window. Its pending774 // A detached target has no primary transport or ACK window. Its pending
763 // state belongs exclusively to picker previews and must be consumed after775 // state belongs exclusively to picker previews and must be consumed after
764- // notification rather than accumulating ackDeferred forever.776+ // notification rather than accumulating ackDeferred forever. Do not pass
777+ // its revoked primary lease to preview consumers.
765 headlessPreviewOnly := c.primaryDetachedLocked()778 headlessPreviewOnly := c.primaryDetachedLocked()
779+ lease := c.lease
780+ if headlessPreviewOnly {
781+ lease = nil
782+ }
783+ w := renderWake{reset: c.pendingReset, urgent: c.pendingUrgent, coalesced: c.coalesced, watchdog: watchdog, lease: lease}
766 if !headlessPreviewOnly && (!ready || (c.lease != nil && c.lease.active && !c.lease.ready)) {784 if !headlessPreviewOnly && (!ready || (c.lease != nil && c.lease.active && !c.lease.ready)) {
767 var ackStart *ackBlockedSpan785 var ackStart *ackBlockedSpan
768 if !ready && deadline {786 if !ready && deadline {
@@ -860,8 +878,8 @@ func (c *renderCoordinator) takePendingPreviewsLocked() (func(renderWake), []fun
860 }878 }
861 c.pendingPreview = false879 c.pendingPreview = false
862 previews := make([]func(renderWake), 0, len(c.previewWakes))880 previews := make([]func(renderWake), 0, len(c.previewWakes))
863- for _, fn := range c.previewWakes {881+ for _, subscription := range c.previewWakes {
864- previews = append(previews, fn)882+ previews = append(previews, subscription.fn)
865 }883 }
866 return c.previewWake, previews884 return c.previewWake, previews
867 }885 }
M internal/usecase/daemon/render_coordinator_handshake_test.go +0 -17
@@ -1,29 +1,12 @@
11 package daemon
22
33 import (
4- "runtime"
54 "testing"
65 "time"
76
87 "github.com/stretchr/testify/require"
98 )
109
11-// awaitHandshakeWorker observes the worker completion channel rather than a
12-// scheduler-dependent no-wake window. The generated mock timer's channel is
13-// the only event that advances it.
14-func awaitHandshakeWorker(t *testing.T, done <-chan struct{}) {
15- t.Helper()
16- for range 1_000_000 {
17- select {
18- case <-done:
19- return
20- default:
21- runtime.Gosched()
22- }
23- }
24- t.Fatal("coordinator deadline worker did not complete")
25-}
26-
2710 // Fresh producer state may arrive after route has published an attachment but
2811 // before the transport has accepted Welcome. It must remain pending until the
2912 // exact attachment incarnation completes that handshake.
@@ -1,29 +1,12 @@
1 package daemon1 package daemon
2 2
3 import (3 import (
4- "runtime"
5 "testing"4 "testing"
6 "time"5 "time"
7 6
8 "github.com/stretchr/testify/require"7 "github.com/stretchr/testify/require"
9 )8 )
10 9
11-// awaitHandshakeWorker observes the worker completion channel rather than a
12-// scheduler-dependent no-wake window. The generated mock timer's channel is
13-// the only event that advances it.
14-func awaitHandshakeWorker(t *testing.T, done <-chan struct{}) {
15- t.Helper()
16- for range 1_000_000 {
17- select {
18- case <-done:
19- return
20- default:
21- runtime.Gosched()
22- }
23- }
24- t.Fatal("coordinator deadline worker did not complete")
25-}
26-
27 // Fresh producer state may arrive after route has published an attachment but10 // Fresh producer state may arrive after route has published an attachment but
28 // before the transport has accepted Welcome. It must remain pending until the11 // before the transport has accepted Welcome. It must remain pending until the
29 // exact attachment incarnation completes that handshake.12 // exact attachment incarnation completes that handshake.
M internal/usecase/daemon/render_coordinator_test.go +46 -89
@@ -3,7 +3,6 @@ package daemon
33 import (
44 "os"
55 "path/filepath"
6- "runtime"
76 "sync"
87 "sync/atomic"
98 "testing"
@@ -120,15 +119,7 @@ func TestRenderCoordinatorDetachJoinsSelectedDeadlineWorker(t *testing.T) {
120119 }
121120
122121 close(release)
123- for range 4096 {
124- select {
125- case <-detached:
126- return
127- default:
128- runtime.Gosched()
129- }
130- }
131- t.Fatal("detach did not complete after the deadline worker was released")
122+ awaitTestCompletion(t, detached, "detach did not complete after the deadline worker was released")
132123 }
133124
134125 func TestRenderCoordinatorTimerCallbacksReenterCoordinator(t *testing.T) {
@@ -207,16 +198,7 @@ func (h *coordinatorHarness) armedTimers(t *testing.T) []*coordinatorMockTimer {
207198 // deterministic contract failure, not a slow behavior to poll for.
208199 func awaitWake(t *testing.T, ch chan renderWake) renderWake {
209200 t.Helper()
210- for range 4096 {
211- select {
212- case w := <-ch:
213- return w
214- default:
215- runtime.Gosched()
216- }
217- }
218- t.Fatal("coordinator did not publish a wake after fake-clock advancement")
219- return renderWake{}
201+ return awaitTestValue(t, ch, "coordinator did not publish a wake after fake-clock advancement")
220202 }
221203
222204 func requireNoWake(t *testing.T, ch chan renderWake) {
@@ -230,16 +212,7 @@ func requireNoWake(t *testing.T, ch chan renderWake) {
230212
231213 func awaitInvalidation(t *testing.T, ch chan renderInvalidation) renderInvalidation {
232214 t.Helper()
233- for range 4096 {
234- select {
235- case inv := <-ch:
236- return inv
237- default:
238- runtime.Gosched()
239- }
240- }
241- t.Fatal("producer did not publish a coordinator invalidation")
242- return renderInvalidation{}
215+ return awaitTestValue(t, ch, "producer did not publish a coordinator invalidation")
243216 }
244217
245218 func requireNoInvalidation(t *testing.T, ch chan renderInvalidation) {
@@ -444,40 +417,16 @@ func TestRenderCoordinatorAckReadinessReentersWithoutBlockingResize(t *testing.T
444417 rc.fire(1, false, true)
445418 close(fireDone)
446419 }()
447- for range 4096 {
448- select {
449- case <-ackEntered:
450- goto resize
451- default:
452- runtime.Gosched()
453- }
454- }
455- t.Fatal("ack readiness was not evaluated")
420+ awaitTestCompletion(t, ackEntered, "ack readiness was not evaluated")
456421
457-resize:
458422 go func() { resizeDone <- rc.recordResizeRequest(domain.Size{Cols: 120, Rows: 40}, owner) }()
459- for range 4096 {
460- select {
461- case epoch := <-resizeDone:
462- require.Equal(t, uint64(1), epoch)
463- wake := awaitWake(t, wakes)
464- require.Same(t, owner, wake.lease.attachment)
465- wake.lease = nil
466- require.Equal(t, renderWake{coalesced: 1}, wake)
467- for range 4096 {
468- select {
469- case <-fireDone:
470- return
471- default:
472- runtime.Gosched()
473- }
474- }
475- t.Fatal("fire did not return after wake")
476- default:
477- runtime.Gosched()
478- }
479- }
480- t.Fatal("ack readiness re-entry blocked resize metadata")
423+ epoch := awaitTestValue(t, resizeDone, "ack readiness re-entry blocked resize metadata")
424+ require.Equal(t, uint64(1), epoch)
425+ wake := awaitWake(t, wakes)
426+ require.Same(t, owner, wake.lease.attachment)
427+ wake.lease = nil
428+ require.Equal(t, renderWake{coalesced: 1}, wake)
429+ awaitTestCompletion(t, fireDone, "fire did not return after wake")
481430 }
482431
483432 func TestRenderCoordinatorAckDoesNotBypassAnUnexpiredDeadline(t *testing.T) {
@@ -704,17 +653,8 @@ func TestRenderCoordinatorSyncGateRetriesWhenBatchPublishesDuringPredicate(t *te
704653 // pending consumption. A stale gate result must be rejected and retried.
705654 h.rc.noteSyncBeginWithRenderability(second, 2, func() bool { return true })
706655 close(releaseGate)
707- for range 4096 {
708- select {
709- case <-fireDone:
710- goto fired
711- default:
712- runtime.Gosched()
713- }
714- }
715- t.Fatal("fire did not finish after retrying the registry snapshot")
656+ awaitTestCompletion(t, fireDone, "fire did not finish after retrying the registry snapshot")
716657
717-fired:
718658 requireNoWake(t, h.wakes)
719659 h.rc.noteSyncEnd(second, 2)
720660 require.Equal(t, renderWake{urgent: true, coalesced: 1}, awaitWake(t, h.wakes))
@@ -844,7 +784,7 @@ func TestRenderCoordinatorPreviewWakesDoNotWaitForTargetAck(t *testing.T) {
844784 h.rc.attach(&attachedClient{})
845785 }
846786 viewer := &attachedClient{}
847- h.rc.subscribePreviewFor(viewer, func(w renderWake) { h.previews <- w })
787+ h.rc.subscribePreviewFor(viewer, 1, func(w renderWake) { h.previews <- w })
848788
849789 h.rc.invalidate(renderInvalidation{class: invalidateOutput, producer: "target output"})
850790 timers := h.armedTimers(t)
@@ -880,7 +820,7 @@ func TestRenderCoordinatorDetachedTargetPublishesPreviewOnlyInvalidations(t *tes
880820 target, viewer := &attachedClient{}, &attachedClient{}
881821 h.ackReady.Store(false)
882822 h.rc.attach(target)
883- h.rc.subscribePreviewFor(viewer, func(w renderWake) { h.previews <- w })
823+ h.rc.subscribePreviewFor(viewer, 1, func(w renderWake) { h.previews <- w })
884824 h.rc.noteDetach(target)
885825
886826 // A detached target only accepts PTY/session-owned invalidations. The old
@@ -894,7 +834,7 @@ func TestRenderCoordinatorDetachedTargetPublishesPreviewOnlyInvalidations(t *tes
894834 require.Len(t, timers, 1)
895835 timers[0].ch <- time.Time{}
896836 preview := awaitWake(t, h.previews)
897- preview.lease = nil
837+ require.Nil(t, preview.lease, "headless preview wakes must not retain a revoked primary lease")
898838 require.Equal(t, renderWake{coalesced: 1}, preview)
899839 requireNoWake(t, h.wakes)
900840
@@ -937,7 +877,7 @@ func TestRenderCoordinatorPreviewLifecycleDropsStaleTargetWakes(t *testing.T) {
937877 h := newCoordinatorHarness(t)
938878 target, replacement, viewer := &attachedClient{}, &attachedClient{}, &attachedClient{}
939879 h.rc.attach(target)
940- h.rc.subscribePreviewFor(viewer, func(w renderWake) { h.previews <- w })
880+ h.rc.subscribePreviewFor(viewer, 1, func(w renderWake) { h.previews <- w })
941881 h.rc.invalidate(renderInvalidation{class: invalidateOutput, producer: "target output"})
942882 stale := h.armedTimers(t)
943883 require.Len(t, stale, 1)
@@ -954,8 +894,8 @@ func TestRenderCoordinatorPreviewSubscriptionsAreIndependent(t *testing.T) {
954894 h := newCoordinatorHarness(t)
955895 one, two := &attachedClient{}, &attachedClient{}
956896 first, second := make(chan renderWake, 1), make(chan renderWake, 1)
957- h.rc.subscribePreviewFor(one, func(w renderWake) { first <- w })
958- h.rc.subscribePreviewFor(two, func(w renderWake) { second <- w })
897+ h.rc.subscribePreviewFor(one, 1, func(w renderWake) { first <- w })
898+ h.rc.subscribePreviewFor(two, 1, func(w renderWake) { second <- w })
959899 h.rc.attach(&attachedClient{})
960900 h.rc.invalidate(renderInvalidation{class: invalidateOutput, producer: "preview"})
961901 last := h.armedTimers(t)
@@ -963,7 +903,7 @@ func TestRenderCoordinatorPreviewSubscriptionsAreIndependent(t *testing.T) {
963903 awaitWake(t, first)
964904 awaitWake(t, second)
965905
966- h.rc.teardownPreviewFor(one)
906+ h.rc.teardownPreviewFor(one, 1)
967907 h.rc.invalidate(renderInvalidation{class: invalidateOutput, producer: "preview"})
968908 last = h.armedTimers(t)
969909 last[len(last)-1].ch <- time.Time{}
@@ -1336,15 +1276,7 @@ func TestStartPaneGoroutinesAccountsForOneReader(t *testing.T) {
13361276
13371277 func requireWorkerExit(t *testing.T, done <-chan struct{}) {
13381278 t.Helper()
1339- for range 4096 {
1340- select {
1341- case <-done:
1342- return
1343- default:
1344- runtime.Gosched()
1345- }
1346- }
1347- t.Fatal("cancelled coordinator timer worker did not exit")
1279+ awaitTestCompletion(t, done, "cancelled coordinator timer worker did not exit")
13481280 }
13491281
13501282 func TestCoordinatorDeadlineCannotPaintPublishedReplacementBeforeOwnershipInstall(t *testing.T) {
@@ -1405,6 +1337,31 @@ func TestRenderCoordinatorClearResizeTimerKeepsNewerTimer(t *testing.T) {
14051337 rc.mu.Unlock()
14061338 }
14071339
1340+func TestRenderCoordinatorNilClockRetryCompletesWithoutSynchronousRun(t *testing.T) {
1341+ rc := newRenderCoordinator(renderCoordinatorOptions{})
1342+ owner := &attachedClient{}
1343+ rc.attach(owner)
1344+ lease := rc.attachmentLease(owner)
1345+ epoch := rc.recordResizeRequestForLease(domain.Size{Cols: 120, Rows: 40}, owner, lease)
1346+ require.NotZero(t, epoch)
1347+ require.True(t, rc.resizeCurrentForLease(epoch, owner, lease, true))
1348+
1349+ called := false
1350+ rc.scheduleResizeRetryForLease(epoch, owner, lease, func() { called = true })
1351+ require.False(t, called, "a nil clock is a disabled retry lane, not a synchronous retry")
1352+ rc.mu.Lock()
1353+ retryTimer, retryCancel, retryDone := rc.retryTimer, rc.retryCancel, rc.retryDone
1354+ rc.mu.Unlock()
1355+ require.Nil(t, retryTimer)
1356+ require.Nil(t, retryCancel)
1357+ require.NotNil(t, retryDone)
1358+ select {
1359+ case <-retryDone:
1360+ default:
1361+ t.Fatal("nil-clock retry did not publish its completion edge")
1362+ }
1363+}
1364+
14081365 func TestRenderCoordinatorInertTimerFiresSynchronouslyWithoutWorker(t *testing.T) {
14091366 clock := portsmocks.NewMockClock(t)
14101367 timer := portsmocks.NewMockTimer(t)
@@ -1442,7 +1399,7 @@ func TestRenderCoordinatorSyncBatchSurvivesAttachmentLifecycle(t *testing.T) {
14421399 h := newCoordinatorHarness(t)
14431400 target, viewer, p := &attachedClient{}, &attachedClient{}, &pane{}
14441401 h.rc.attach(target)
1445- h.rc.subscribePreviewFor(viewer, func(w renderWake) { h.previews <- w })
1402+ h.rc.subscribePreviewFor(viewer, 1, func(w renderWake) { h.previews <- w })
14461403 h.rc.noteSyncBegin(p, 1)
14471404 watchdog := h.armedTimers(t)[0]
14481405 transition.run(h.rc, target)
@@ -3,7 +3,6 @@ package daemon
3 import (3 import (
4 "os"4 "os"
5 "path/filepath"5 "path/filepath"
6- "runtime"
7 "sync"6 "sync"
8 "sync/atomic"7 "sync/atomic"
9 "testing"8 "testing"
@@ -120,15 +119,7 @@ func TestRenderCoordinatorDetachJoinsSelectedDeadlineWorker(t *testing.T) {
120 }119 }
121 120
122 close(release)121 close(release)
123- for range 4096 {122+ awaitTestCompletion(t, detached, "detach did not complete after the deadline worker was released")
124- select {
125- case <-detached:
126- return
127- default:
128- runtime.Gosched()
129- }
130- }
131- t.Fatal("detach did not complete after the deadline worker was released")
132 }123 }
133 124
134 func TestRenderCoordinatorTimerCallbacksReenterCoordinator(t *testing.T) {125 func TestRenderCoordinatorTimerCallbacksReenterCoordinator(t *testing.T) {
@@ -207,16 +198,7 @@ func (h *coordinatorHarness) armedTimers(t *testing.T) []*coordinatorMockTimer {
207 // deterministic contract failure, not a slow behavior to poll for.198 // deterministic contract failure, not a slow behavior to poll for.
208 func awaitWake(t *testing.T, ch chan renderWake) renderWake {199 func awaitWake(t *testing.T, ch chan renderWake) renderWake {
209 t.Helper()200 t.Helper()
210- for range 4096 {201+ return awaitTestValue(t, ch, "coordinator did not publish a wake after fake-clock advancement")
211- select {
212- case w := <-ch:
213- return w
214- default:
215- runtime.Gosched()
216- }
217- }
218- t.Fatal("coordinator did not publish a wake after fake-clock advancement")
219- return renderWake{}
220 }202 }
221 203
222 func requireNoWake(t *testing.T, ch chan renderWake) {204 func requireNoWake(t *testing.T, ch chan renderWake) {
@@ -230,16 +212,7 @@ func requireNoWake(t *testing.T, ch chan renderWake) {
230 212
231 func awaitInvalidation(t *testing.T, ch chan renderInvalidation) renderInvalidation {213 func awaitInvalidation(t *testing.T, ch chan renderInvalidation) renderInvalidation {
232 t.Helper()214 t.Helper()
233- for range 4096 {215+ return awaitTestValue(t, ch, "producer did not publish a coordinator invalidation")
234- select {
235- case inv := <-ch:
236- return inv
237- default:
238- runtime.Gosched()
239- }
240- }
241- t.Fatal("producer did not publish a coordinator invalidation")
242- return renderInvalidation{}
243 }216 }
244 217
245 func requireNoInvalidation(t *testing.T, ch chan renderInvalidation) {218 func requireNoInvalidation(t *testing.T, ch chan renderInvalidation) {
@@ -444,40 +417,16 @@ func TestRenderCoordinatorAckReadinessReentersWithoutBlockingResize(t *testing.T
444 rc.fire(1, false, true)417 rc.fire(1, false, true)
445 close(fireDone)418 close(fireDone)
446 }()419 }()
447- for range 4096 {420+ awaitTestCompletion(t, ackEntered, "ack readiness was not evaluated")
448- select {
449- case <-ackEntered:
450- goto resize
451- default:
452- runtime.Gosched()
453- }
454- }
455- t.Fatal("ack readiness was not evaluated")
456 421
457-resize:
458 go func() { resizeDone <- rc.recordResizeRequest(domain.Size{Cols: 120, Rows: 40}, owner) }()422 go func() { resizeDone <- rc.recordResizeRequest(domain.Size{Cols: 120, Rows: 40}, owner) }()
459- for range 4096 {423+ epoch := awaitTestValue(t, resizeDone, "ack readiness re-entry blocked resize metadata")
460- select {424+ require.Equal(t, uint64(1), epoch)
461- case epoch := <-resizeDone:425+ wake := awaitWake(t, wakes)
462- require.Equal(t, uint64(1), epoch)426+ require.Same(t, owner, wake.lease.attachment)
463- wake := awaitWake(t, wakes)427+ wake.lease = nil
464- require.Same(t, owner, wake.lease.attachment)428+ require.Equal(t, renderWake{coalesced: 1}, wake)
465- wake.lease = nil429+ awaitTestCompletion(t, fireDone, "fire did not return after wake")
466- require.Equal(t, renderWake{coalesced: 1}, wake)
467- for range 4096 {
468- select {
469- case <-fireDone:
470- return
471- default:
472- runtime.Gosched()
473- }
474- }
475- t.Fatal("fire did not return after wake")
476- default:
477- runtime.Gosched()
478- }
479- }
480- t.Fatal("ack readiness re-entry blocked resize metadata")
481 }430 }
482 431
483 func TestRenderCoordinatorAckDoesNotBypassAnUnexpiredDeadline(t *testing.T) {432 func TestRenderCoordinatorAckDoesNotBypassAnUnexpiredDeadline(t *testing.T) {
@@ -704,17 +653,8 @@ func TestRenderCoordinatorSyncGateRetriesWhenBatchPublishesDuringPredicate(t *te
704 // pending consumption. A stale gate result must be rejected and retried.653 // pending consumption. A stale gate result must be rejected and retried.
705 h.rc.noteSyncBeginWithRenderability(second, 2, func() bool { return true })654 h.rc.noteSyncBeginWithRenderability(second, 2, func() bool { return true })
706 close(releaseGate)655 close(releaseGate)
707- for range 4096 {656+ awaitTestCompletion(t, fireDone, "fire did not finish after retrying the registry snapshot")
708- select {
709- case <-fireDone:
710- goto fired
711- default:
712- runtime.Gosched()
713- }
714- }
715- t.Fatal("fire did not finish after retrying the registry snapshot")
716 657
717-fired:
718 requireNoWake(t, h.wakes)658 requireNoWake(t, h.wakes)
719 h.rc.noteSyncEnd(second, 2)659 h.rc.noteSyncEnd(second, 2)
720 require.Equal(t, renderWake{urgent: true, coalesced: 1}, awaitWake(t, h.wakes))660 require.Equal(t, renderWake{urgent: true, coalesced: 1}, awaitWake(t, h.wakes))
@@ -844,7 +784,7 @@ func TestRenderCoordinatorPreviewWakesDoNotWaitForTargetAck(t *testing.T) {
844 h.rc.attach(&attachedClient{})784 h.rc.attach(&attachedClient{})
845 }785 }
846 viewer := &attachedClient{}786 viewer := &attachedClient{}
847- h.rc.subscribePreviewFor(viewer, func(w renderWake) { h.previews <- w })787+ h.rc.subscribePreviewFor(viewer, 1, func(w renderWake) { h.previews <- w })
848 788
849 h.rc.invalidate(renderInvalidation{class: invalidateOutput, producer: "target output"})789 h.rc.invalidate(renderInvalidation{class: invalidateOutput, producer: "target output"})
850 timers := h.armedTimers(t)790 timers := h.armedTimers(t)
@@ -880,7 +820,7 @@ func TestRenderCoordinatorDetachedTargetPublishesPreviewOnlyInvalidations(t *tes
880 target, viewer := &attachedClient{}, &attachedClient{}820 target, viewer := &attachedClient{}, &attachedClient{}
881 h.ackReady.Store(false)821 h.ackReady.Store(false)
882 h.rc.attach(target)822 h.rc.attach(target)
883- h.rc.subscribePreviewFor(viewer, func(w renderWake) { h.previews <- w })823+ h.rc.subscribePreviewFor(viewer, 1, func(w renderWake) { h.previews <- w })
884 h.rc.noteDetach(target)824 h.rc.noteDetach(target)
885 825
886 // A detached target only accepts PTY/session-owned invalidations. The old826 // A detached target only accepts PTY/session-owned invalidations. The old
@@ -894,7 +834,7 @@ func TestRenderCoordinatorDetachedTargetPublishesPreviewOnlyInvalidations(t *tes
894 require.Len(t, timers, 1)834 require.Len(t, timers, 1)
895 timers[0].ch <- time.Time{}835 timers[0].ch <- time.Time{}
896 preview := awaitWake(t, h.previews)836 preview := awaitWake(t, h.previews)
897- preview.lease = nil837+ require.Nil(t, preview.lease, "headless preview wakes must not retain a revoked primary lease")
898 require.Equal(t, renderWake{coalesced: 1}, preview)838 require.Equal(t, renderWake{coalesced: 1}, preview)
899 requireNoWake(t, h.wakes)839 requireNoWake(t, h.wakes)
900 840
@@ -937,7 +877,7 @@ func TestRenderCoordinatorPreviewLifecycleDropsStaleTargetWakes(t *testing.T) {
937 h := newCoordinatorHarness(t)877 h := newCoordinatorHarness(t)
938 target, replacement, viewer := &attachedClient{}, &attachedClient{}, &attachedClient{}878 target, replacement, viewer := &attachedClient{}, &attachedClient{}, &attachedClient{}
939 h.rc.attach(target)879 h.rc.attach(target)
940- h.rc.subscribePreviewFor(viewer, func(w renderWake) { h.previews <- w })880+ h.rc.subscribePreviewFor(viewer, 1, func(w renderWake) { h.previews <- w })
941 h.rc.invalidate(renderInvalidation{class: invalidateOutput, producer: "target output"})881 h.rc.invalidate(renderInvalidation{class: invalidateOutput, producer: "target output"})
942 stale := h.armedTimers(t)882 stale := h.armedTimers(t)
943 require.Len(t, stale, 1)883 require.Len(t, stale, 1)
@@ -954,8 +894,8 @@ func TestRenderCoordinatorPreviewSubscriptionsAreIndependent(t *testing.T) {
954 h := newCoordinatorHarness(t)894 h := newCoordinatorHarness(t)
955 one, two := &attachedClient{}, &attachedClient{}895 one, two := &attachedClient{}, &attachedClient{}
956 first, second := make(chan renderWake, 1), make(chan renderWake, 1)896 first, second := make(chan renderWake, 1), make(chan renderWake, 1)
957- h.rc.subscribePreviewFor(one, func(w renderWake) { first <- w })897+ h.rc.subscribePreviewFor(one, 1, func(w renderWake) { first <- w })
958- h.rc.subscribePreviewFor(two, func(w renderWake) { second <- w })898+ h.rc.subscribePreviewFor(two, 1, func(w renderWake) { second <- w })
959 h.rc.attach(&attachedClient{})899 h.rc.attach(&attachedClient{})
960 h.rc.invalidate(renderInvalidation{class: invalidateOutput, producer: "preview"})900 h.rc.invalidate(renderInvalidation{class: invalidateOutput, producer: "preview"})
961 last := h.armedTimers(t)901 last := h.armedTimers(t)
@@ -963,7 +903,7 @@ func TestRenderCoordinatorPreviewSubscriptionsAreIndependent(t *testing.T) {
963 awaitWake(t, first)903 awaitWake(t, first)
964 awaitWake(t, second)904 awaitWake(t, second)
965 905
966- h.rc.teardownPreviewFor(one)906+ h.rc.teardownPreviewFor(one, 1)
967 h.rc.invalidate(renderInvalidation{class: invalidateOutput, producer: "preview"})907 h.rc.invalidate(renderInvalidation{class: invalidateOutput, producer: "preview"})
968 last = h.armedTimers(t)908 last = h.armedTimers(t)
969 last[len(last)-1].ch <- time.Time{}909 last[len(last)-1].ch <- time.Time{}
@@ -1336,15 +1276,7 @@ func TestStartPaneGoroutinesAccountsForOneReader(t *testing.T) {
1336 1276
1337 func requireWorkerExit(t *testing.T, done <-chan struct{}) {1277 func requireWorkerExit(t *testing.T, done <-chan struct{}) {
1338 t.Helper()1278 t.Helper()
1339- for range 4096 {1279+ awaitTestCompletion(t, done, "cancelled coordinator timer worker did not exit")
1340- select {
1341- case <-done:
1342- return
1343- default:
1344- runtime.Gosched()
1345- }
1346- }
1347- t.Fatal("cancelled coordinator timer worker did not exit")
1348 }1280 }
1349 1281
1350 func TestCoordinatorDeadlineCannotPaintPublishedReplacementBeforeOwnershipInstall(t *testing.T) {1282 func TestCoordinatorDeadlineCannotPaintPublishedReplacementBeforeOwnershipInstall(t *testing.T) {
@@ -1405,6 +1337,31 @@ func TestRenderCoordinatorClearResizeTimerKeepsNewerTimer(t *testing.T) {
1405 rc.mu.Unlock()1337 rc.mu.Unlock()
1406 }1338 }
1407 1339
1340+func TestRenderCoordinatorNilClockRetryCompletesWithoutSynchronousRun(t *testing.T) {
1341+ rc := newRenderCoordinator(renderCoordinatorOptions{})
1342+ owner := &attachedClient{}
1343+ rc.attach(owner)
1344+ lease := rc.attachmentLease(owner)
1345+ epoch := rc.recordResizeRequestForLease(domain.Size{Cols: 120, Rows: 40}, owner, lease)
1346+ require.NotZero(t, epoch)
1347+ require.True(t, rc.resizeCurrentForLease(epoch, owner, lease, true))
1348+
1349+ called := false
1350+ rc.scheduleResizeRetryForLease(epoch, owner, lease, func() { called = true })
1351+ require.False(t, called, "a nil clock is a disabled retry lane, not a synchronous retry")
1352+ rc.mu.Lock()
1353+ retryTimer, retryCancel, retryDone := rc.retryTimer, rc.retryCancel, rc.retryDone
1354+ rc.mu.Unlock()
1355+ require.Nil(t, retryTimer)
1356+ require.Nil(t, retryCancel)
1357+ require.NotNil(t, retryDone)
1358+ select {
1359+ case <-retryDone:
1360+ default:
1361+ t.Fatal("nil-clock retry did not publish its completion edge")
1362+ }
1363+}
1364+
1408 func TestRenderCoordinatorInertTimerFiresSynchronouslyWithoutWorker(t *testing.T) {1365 func TestRenderCoordinatorInertTimerFiresSynchronouslyWithoutWorker(t *testing.T) {
1409 clock := portsmocks.NewMockClock(t)1366 clock := portsmocks.NewMockClock(t)
1410 timer := portsmocks.NewMockTimer(t)1367 timer := portsmocks.NewMockTimer(t)
@@ -1442,7 +1399,7 @@ func TestRenderCoordinatorSyncBatchSurvivesAttachmentLifecycle(t *testing.T) {
1442 h := newCoordinatorHarness(t)1399 h := newCoordinatorHarness(t)
1443 target, viewer, p := &attachedClient{}, &attachedClient{}, &pane{}1400 target, viewer, p := &attachedClient{}, &attachedClient{}, &pane{}
1444 h.rc.attach(target)1401 h.rc.attach(target)
1445- h.rc.subscribePreviewFor(viewer, func(w renderWake) { h.previews <- w })1402+ h.rc.subscribePreviewFor(viewer, 1, func(w renderWake) { h.previews <- w })
1446 h.rc.noteSyncBegin(p, 1)1403 h.rc.noteSyncBegin(p, 1)
1447 watchdog := h.armedTimers(t)[0]1404 watchdog := h.armedTimers(t)[0]
1448 transition.run(h.rc, target)1405 transition.run(h.rc, target)
M internal/usecase/daemon/render_coordinator_types.go +7 -13
@@ -12,13 +12,9 @@ import (
1212 // Coordinator contracts and support types are kept with their ownership
1313 // semantics, separate from the scheduling transitions in render_coordinator.
1414
15-// S1 render coordinator contract. One coordinator serves one attached
16-// session: producers mutate authoritative state and publish an invalidation;
17-// only the coordinator decides when the transitional composition target runs.
18-// Producers never compose or send. This file currently carries the inert
19-// contract — state, options, and signatures exist so the behavioral
20-// specification compiles — while wake scheduling, ACK gating, synchronized
21-// output handling, and producer migration land in the GREEN slice.
15+// One coordinator serves a session: producers mutate authoritative state and
16+// publish an invalidation; only the coordinator decides when composition runs.
17+// Producers never compose or send.
2218
2319 // Coordinator deadline bounds. Urgent transitions (input echo, overlay state
2420 // changes) must become visible within urgentRenderDeadline; bulk output
@@ -50,8 +46,7 @@ type renderInvalidation struct {
5046 producer string
5147 }
5248
53-// renderWake is one coalesced composition request delivered to the
54-// transitional composition target (replaced by the S2 pipeline).
49+// renderWake is one coalesced composition request.
5550 type renderWake struct {
5651 reset bool
5752 urgent bool
@@ -78,9 +73,8 @@ type attachmentLease struct {
7873 }
7974
8075 // resizeRequestMetadata is the coordinator-owned latest requested resize
81-// state: S3's transaction entry point reads it through resizeSnapshot. It is
82-// metadata ownership only — S1/S2 dispatch stays with the coordinator
83-// epoch timer path.
76+// state. Resize dispatch reads it through resizeSnapshot and validates the
77+// coordinator epoch before applying it.
8478 type resizeRequestMetadata struct {
8579 size domain.Size
8680 source *attachedClient
@@ -95,7 +89,7 @@ type resizeRequestMetadata struct {
9589 type renderCoordinatorOptions struct {
9690 clock ports.Clock
9791 observer ports.RuntimeObserver
98- // wake is the transitional composition target.
92+ // wake composes a coalesced render request.
9993 wake func(renderWake)
10094 // ackReady reports whether the attachment may compose another output
10195 // state (the outputStateStream window has capacity).
@@ -12,13 +12,9 @@ import (
12 // Coordinator contracts and support types are kept with their ownership12 // Coordinator contracts and support types are kept with their ownership
13 // semantics, separate from the scheduling transitions in render_coordinator.13 // semantics, separate from the scheduling transitions in render_coordinator.
14 14
15-// S1 render coordinator contract. One coordinator serves one attached15+// One coordinator serves a session: producers mutate authoritative state and
16-// session: producers mutate authoritative state and publish an invalidation;16+// publish an invalidation; only the coordinator decides when composition runs.
17-// only the coordinator decides when the transitional composition target runs.17+// Producers never compose or send.
18-// Producers never compose or send. This file currently carries the inert
19-// contract — state, options, and signatures exist so the behavioral
20-// specification compiles — while wake scheduling, ACK gating, synchronized
21-// output handling, and producer migration land in the GREEN slice.
22 18
23 // Coordinator deadline bounds. Urgent transitions (input echo, overlay state19 // Coordinator deadline bounds. Urgent transitions (input echo, overlay state
24 // changes) must become visible within urgentRenderDeadline; bulk output20 // changes) must become visible within urgentRenderDeadline; bulk output
@@ -50,8 +46,7 @@ type renderInvalidation struct {
50 producer string46 producer string
51 }47 }
52 48
53-// renderWake is one coalesced composition request delivered to the49+// renderWake is one coalesced composition request.
54-// transitional composition target (replaced by the S2 pipeline).
55 type renderWake struct {50 type renderWake struct {
56 reset bool51 reset bool
57 urgent bool52 urgent bool
@@ -78,9 +73,8 @@ type attachmentLease struct {
78 }73 }
79 74
80 // resizeRequestMetadata is the coordinator-owned latest requested resize75 // resizeRequestMetadata is the coordinator-owned latest requested resize
81-// state: S3's transaction entry point reads it through resizeSnapshot. It is76+// state. Resize dispatch reads it through resizeSnapshot and validates the
82-// metadata ownership only — S1/S2 dispatch stays with the coordinator77+// coordinator epoch before applying it.
83-// epoch timer path.
84 type resizeRequestMetadata struct {78 type resizeRequestMetadata struct {
85 size domain.Size79 size domain.Size
86 source *attachedClient80 source *attachedClient
@@ -95,7 +89,7 @@ type resizeRequestMetadata struct {
95 type renderCoordinatorOptions struct {89 type renderCoordinatorOptions struct {
96 clock ports.Clock90 clock ports.Clock
97 observer ports.RuntimeObserver91 observer ports.RuntimeObserver
98- // wake is the transitional composition target.92+ // wake composes a coalesced render request.
99 wake func(renderWake)93 wake func(renderWake)
100 // ackReady reports whether the attachment may compose another output94 // ackReady reports whether the attachment may compose another output
101 // state (the outputStateStream window has capacity).95 // state (the outputStateStream window has capacity).
M internal/usecase/daemon/render_resize_deadline.go +2 -3
@@ -128,12 +128,11 @@ func (c *renderCoordinator) scheduleResizeRetryForLease(epoch uint64, source *at
128128 c.mu.Unlock()
129129 stopAndJoinTimerWorker(old, nil)
130130 if clock == nil {
131+ // A nil clock disables deferred retries. Completing the generation keeps
132+ // retry bookkeeping consistent without recursively re-entering resize.
131133 c.mu.Lock()
132134 c.retryTimerOwner().completeLocked(gen)
133135 c.mu.Unlock()
134- if c.retryCurrentForLease(epoch, source, lease) {
135- run()
136- }
137136 return
138137 }
139138 timer := clock.NewTimer(minOutputRenderDeadline)
@@ -128,12 +128,11 @@ func (c *renderCoordinator) scheduleResizeRetryForLease(epoch uint64, source *at
128 c.mu.Unlock()128 c.mu.Unlock()
129 stopAndJoinTimerWorker(old, nil)129 stopAndJoinTimerWorker(old, nil)
130 if clock == nil {130 if clock == nil {
131+ // A nil clock disables deferred retries. Completing the generation keeps
132+ // retry bookkeeping consistent without recursively re-entering resize.
131 c.mu.Lock()133 c.mu.Lock()
132 c.retryTimerOwner().completeLocked(gen)134 c.retryTimerOwner().completeLocked(gen)
133 c.mu.Unlock()135 c.mu.Unlock()
134- if c.retryCurrentForLease(epoch, source, lease) {
135- run()
136- }
137 return136 return
138 }137 }
139 timer := clock.NewTimer(minOutputRenderDeadline)138 timer := clock.NewTimer(minOutputRenderDeadline)
M internal/usecase/daemon/render_test.go +17 -1
@@ -405,7 +405,7 @@ func TestPTYReaderRepublishesSynchronizedCompletionAfterAttachmentLifecycle(t *t
405405 viewer.overlays.pickerMu.Unlock()
406406 d.sessions["viewer"] = &session{id: "viewer", client: viewer}
407407 previews := make(chan renderWake, 2)
408- rc.subscribePreviewFor(viewer, func(w renderWake) { previews <- w })
408+ rc.subscribePreviewFor(viewer, 1, func(w renderWake) { previews <- w })
409409
410410 d.sessWg.Add(1)
411411 go d.ptyReader(target, target.tabs[0], target.tabs[0].focusedPane())
@@ -509,6 +509,22 @@ func TestPTYReaderRepublishesSynchronizedCompletionAfterAttachmentLifecycle(t *t
509509 // S2 keeps damage pending until the owning render capture consumes it. In
510510 // particular, a pane becoming invisible must not let a PTY reader erase data
511511 // that a later attachment or picker preview needs to render.
512+func TestPaneRenderableActiveAttachmentDoesNotScanPickerPreviews(t *testing.T) {
513+ p, release := newBlockingPTY(t)
514+ defer release()
515+ d, sess, _, _ := newManualSessionWithPTYs(t, p)
516+ tb := sess.activeTab()
517+ pane := tb.focusedPane()
518+
519+ // Holding daemon ownership makes a picker scan block. An active attached
520+ // tab must decide renderability from its session state without that scan.
521+ d.mu.Lock()
522+ defer d.mu.Unlock()
523+ done := make(chan bool, 1)
524+ go func() { done <- d.paneRenderable(sess, tb, pane) }()
525+ require.True(t, awaitTestValue(t, done, "active attached renderability scanned picker previews"))
526+}
527+
512528 func TestNonRenderablePaneDamageRemainsPendingForCapture(t *testing.T) {
513529 newFixture := func(t *testing.T) (*Daemon, *session, *tab, *pane, chan ports.Frame) {
514530 t.Helper()
@@ -405,7 +405,7 @@ func TestPTYReaderRepublishesSynchronizedCompletionAfterAttachmentLifecycle(t *t
405 viewer.overlays.pickerMu.Unlock()405 viewer.overlays.pickerMu.Unlock()
406 d.sessions["viewer"] = &session{id: "viewer", client: viewer}406 d.sessions["viewer"] = &session{id: "viewer", client: viewer}
407 previews := make(chan renderWake, 2)407 previews := make(chan renderWake, 2)
408- rc.subscribePreviewFor(viewer, func(w renderWake) { previews <- w })408+ rc.subscribePreviewFor(viewer, 1, func(w renderWake) { previews <- w })
409 409
410 d.sessWg.Add(1)410 d.sessWg.Add(1)
411 go d.ptyReader(target, target.tabs[0], target.tabs[0].focusedPane())411 go d.ptyReader(target, target.tabs[0], target.tabs[0].focusedPane())
@@ -509,6 +509,22 @@ func TestPTYReaderRepublishesSynchronizedCompletionAfterAttachmentLifecycle(t *t
509 // S2 keeps damage pending until the owning render capture consumes it. In509 // S2 keeps damage pending until the owning render capture consumes it. In
510 // particular, a pane becoming invisible must not let a PTY reader erase data510 // particular, a pane becoming invisible must not let a PTY reader erase data
511 // that a later attachment or picker preview needs to render.511 // that a later attachment or picker preview needs to render.
512+func TestPaneRenderableActiveAttachmentDoesNotScanPickerPreviews(t *testing.T) {
513+ p, release := newBlockingPTY(t)
514+ defer release()
515+ d, sess, _, _ := newManualSessionWithPTYs(t, p)
516+ tb := sess.activeTab()
517+ pane := tb.focusedPane()
518+
519+ // Holding daemon ownership makes a picker scan block. An active attached
520+ // tab must decide renderability from its session state without that scan.
521+ d.mu.Lock()
522+ defer d.mu.Unlock()
523+ done := make(chan bool, 1)
524+ go func() { done <- d.paneRenderable(sess, tb, pane) }()
525+ require.True(t, awaitTestValue(t, done, "active attached renderability scanned picker previews"))
526+}
527+
512 func TestNonRenderablePaneDamageRemainsPendingForCapture(t *testing.T) {528 func TestNonRenderablePaneDamageRemainsPendingForCapture(t *testing.T) {
513 newFixture := func(t *testing.T) (*Daemon, *session, *tab, *pane, chan ports.Frame) {529 newFixture := func(t *testing.T) (*Daemon, *session, *tab, *pane, chan ports.Frame) {
514 t.Helper()530 t.Helper()
M internal/usecase/daemon/render_timer_test.go +2 -16
@@ -1,7 +1,6 @@
11 package daemon
22
33 import (
4- "runtime"
54 "testing"
65 "time"
76
@@ -36,7 +35,7 @@ func TestTimerOwnershipRejectsStaleTicketsAndCancelsWorkers(t *testing.T) {
3635 require.False(t, ok, "a stale generation must not reclaim a replacement lane")
3736
3837 require.True(t, owner.completeLocked(generation))
39- awaitTimerWorker(t, done)
38+ awaitTestCompletion(t, done, "timer worker did not complete")
4039 }
4140
4241 func TestRunTimerWorkerOwnsCompletionAfterTick(t *testing.T) {
@@ -46,23 +45,10 @@ func TestRunTimerWorkerOwnsCompletionAfterTick(t *testing.T) {
4645 fired := make(chan struct{}, 1)
4746 runTimerWorker(timerC, cancel, done, func() { fired <- struct{}{} })
4847 timerC <- time.Time{}
49- awaitTimerWorker(t, done)
48+ awaitTestCompletion(t, done, "timer worker did not complete")
5049 select {
5150 case <-fired:
5251 default:
5352 t.Fatal("timer worker did not run callback")
5453 }
5554 }
56-
57-func awaitTimerWorker(t *testing.T, done <-chan struct{}) {
58- t.Helper()
59- for range 4096 {
60- select {
61- case <-done:
62- return
63- default:
64- runtime.Gosched()
65- }
66- }
67- t.Fatal("timer worker did not complete")
68-}
@@ -1,7 +1,6 @@
1 package daemon1 package daemon
2 2
3 import (3 import (
4- "runtime"
5 "testing"4 "testing"
6 "time"5 "time"
7 6
@@ -36,7 +35,7 @@ func TestTimerOwnershipRejectsStaleTicketsAndCancelsWorkers(t *testing.T) {
36 require.False(t, ok, "a stale generation must not reclaim a replacement lane")35 require.False(t, ok, "a stale generation must not reclaim a replacement lane")
37 36
38 require.True(t, owner.completeLocked(generation))37 require.True(t, owner.completeLocked(generation))
39- awaitTimerWorker(t, done)38+ awaitTestCompletion(t, done, "timer worker did not complete")
40 }39 }
41 40
42 func TestRunTimerWorkerOwnsCompletionAfterTick(t *testing.T) {41 func TestRunTimerWorkerOwnsCompletionAfterTick(t *testing.T) {
@@ -46,23 +45,10 @@ func TestRunTimerWorkerOwnsCompletionAfterTick(t *testing.T) {
46 fired := make(chan struct{}, 1)45 fired := make(chan struct{}, 1)
47 runTimerWorker(timerC, cancel, done, func() { fired <- struct{}{} })46 runTimerWorker(timerC, cancel, done, func() { fired <- struct{}{} })
48 timerC <- time.Time{}47 timerC <- time.Time{}
49- awaitTimerWorker(t, done)48+ awaitTestCompletion(t, done, "timer worker did not complete")
50 select {49 select {
51 case <-fired:50 case <-fired:
52 default:51 default:
53 t.Fatal("timer worker did not run callback")52 t.Fatal("timer worker did not run callback")
54 }53 }
55 }54 }
56-
57-func awaitTimerWorker(t *testing.T, done <-chan struct{}) {
58- t.Helper()
59- for range 4096 {
60- select {
61- case <-done:
62- return
63- default:
64- runtime.Gosched()
65- }
66- }
67- t.Fatal("timer worker did not complete")
68-}
M internal/usecase/daemon/resume_test.go +2 -2
@@ -171,10 +171,10 @@ func TestHandleHelloResumeDefersFreshOutputUntilWelcome(t *testing.T) {
171171 rc.mu.Unlock()
172172 require.NotNil(t, workerDone)
173173 timer.ch <- time.Time{}
174- awaitHandshakeWorker(t, workerDone)
174+ awaitTestCompletion(t, workerDone, "coordinator deadline worker did not complete")
175175 requireNoCoordinatorOutputFrame(t, tr.sends)
176176
177- close(tr.releaseWelcome)
177+ tr.release()
178178 output := awaitFrame(t, tr.sends, ports.MsgOutput)
179179 first, err := ports.UnmarshalOutput(output.Payload)
180180 require.NoError(t, err)
@@ -171,10 +171,10 @@ func TestHandleHelloResumeDefersFreshOutputUntilWelcome(t *testing.T) {
171 rc.mu.Unlock()171 rc.mu.Unlock()
172 require.NotNil(t, workerDone)172 require.NotNil(t, workerDone)
173 timer.ch <- time.Time{}173 timer.ch <- time.Time{}
174- awaitHandshakeWorker(t, workerDone)174+ awaitTestCompletion(t, workerDone, "coordinator deadline worker did not complete")
175 requireNoCoordinatorOutputFrame(t, tr.sends)175 requireNoCoordinatorOutputFrame(t, tr.sends)
176 176
177- close(tr.releaseWelcome)177+ tr.release()
178 output := awaitFrame(t, tr.sends, ports.MsgOutput)178 output := awaitFrame(t, tr.sends, ports.MsgOutput)
179 first, err := ports.UnmarshalOutput(output.Payload)179 first, err := ports.UnmarshalOutput(output.Payload)
180 require.NoError(t, err)180 require.NoError(t, err)
M internal/usecase/daemon/snapshot_test.go +3 -1
@@ -624,6 +624,7 @@ func TestSnapshotSaverWritesDirtyNamedSessionsOnly(t *testing.T) {
624624 case <-time.After(time.Second):
625625 t.Fatal("timed out waiting for dirty snapshot write")
626626 }
627+ awaitSnapshotClean(t, dirty)
627628
628629 clock.fireNext(t)
629630 select {
@@ -850,6 +851,7 @@ func TestSnapshotSaverKeepsDirtyWhenWriteFails(t *testing.T) {
850851 case <-time.After(time.Second):
851852 t.Fatal("timed out waiting for retry snapshot write")
852853 }
854+ awaitSnapshotClean(t, sess)
853855 require.False(t, sess.snapDirty.Load())
854856 }
855857
@@ -1037,7 +1039,7 @@ func newSnapshotTestSession(t *testing.T, name string, ephemeral bool, cwd strin
10371039 return sess
10381040 }
10391041
1040-func awaitSnapshotIdle(t *testing.T, sess *session) {
1042+func awaitSnapshotIdle(t testing.TB, sess *session) {
10411043 t.Helper()
10421044 for {
10431045 sess.snapshotMu.Lock()
@@ -624,6 +624,7 @@ func TestSnapshotSaverWritesDirtyNamedSessionsOnly(t *testing.T) {
624 case <-time.After(time.Second):624 case <-time.After(time.Second):
625 t.Fatal("timed out waiting for dirty snapshot write")625 t.Fatal("timed out waiting for dirty snapshot write")
626 }626 }
627+ awaitSnapshotClean(t, dirty)
627 628
628 clock.fireNext(t)629 clock.fireNext(t)
629 select {630 select {
@@ -850,6 +851,7 @@ func TestSnapshotSaverKeepsDirtyWhenWriteFails(t *testing.T) {
850 case <-time.After(time.Second):851 case <-time.After(time.Second):
851 t.Fatal("timed out waiting for retry snapshot write")852 t.Fatal("timed out waiting for retry snapshot write")
852 }853 }
854+ awaitSnapshotClean(t, sess)
853 require.False(t, sess.snapDirty.Load())855 require.False(t, sess.snapDirty.Load())
854 }856 }
855 857
@@ -1037,7 +1039,7 @@ func newSnapshotTestSession(t *testing.T, name string, ephemeral bool, cwd strin
1037 return sess1039 return sess
1038 }1040 }
1039 1041
1040-func awaitSnapshotIdle(t *testing.T, sess *session) {1042+func awaitSnapshotIdle(t testing.TB, sess *session) {
1041 t.Helper()1043 t.Helper()
1042 for {1044 for {
1043 sess.snapshotMu.Lock()1045 sess.snapshotMu.Lock()
A internal/usecase/daemon/stalled_attachment_test.go +78 -0
@@ -0,0 +1,78 @@
1+package daemon
2+
3+import (
4+ "io"
5+ "sync"
6+ "testing"
7+ "time"
8+
9+ "github.com/stretchr/testify/mock"
10+ "github.com/stretchr/testify/require"
11+
12+ "github.com/bnema/vev/internal/domain"
13+ "github.com/bnema/vev/internal/ports"
14+ portsmocks "github.com/bnema/vev/internal/ports/mocks"
15+)
16+
17+// TestFinishAttachDoesNotWaitForBlockedOldRenderWorker verifies the replacement
18+// boundary rather than relying on a scheduler delay: an old coordinator worker
19+// is already blocked in Send when finishAttach publishes its replacement.
20+func TestFinishAttachDoesNotWaitForBlockedOldRenderWorker(t *testing.T) {
21+ pty, releasePTY := newBlockingPTY(t)
22+ defer releasePTY()
23+ clock := newCoordinatorMockClock(t, 4)
24+ d := newTestDaemon(t, newFactory(t, pty), clock.clock)
25+
26+ oldTransport := portsmocks.NewMockTransport(t)
27+ oldSendEntered := make(chan struct{})
28+ oldClosed := make(chan struct{})
29+ var closeOnce sync.Once
30+ oldTransport.EXPECT().Send(mock.Anything).RunAndReturn(func(ports.Frame) error {
31+ close(oldSendEntered)
32+ <-oldClosed
33+ return io.EOF
34+ }).Once()
35+ oldTransport.EXPECT().Close().RunAndReturn(func() error {
36+ closeOnce.Do(func() { close(oldClosed) })
37+ return nil
38+ }).Once()
39+
40+ hello := ports.Hello{
41+ Version: ports.ProtocolVersion,
42+ Intent: ports.IntentNew,
43+ Name: "stalled-attachment",
44+ Size: domain.Size{Cols: 80, Rows: 24},
45+ TermEnv: "xterm-256color",
46+ }
47+ sess, old, err := d.route(hello, oldTransport)
48+ require.NoError(t, err)
49+ rc := sess.renderCoordinator()
50+ lease := rc.attachmentLease(old)
51+ require.True(t, rc.markAttachmentReady(lease))
52+
53+ rc.invalidateForAttachment(old, renderInvalidation{class: invalidateUrgent, reset: true, producer: "test"})
54+ awaitCoordinatorScheduledTimer(t, clock).ch <- time.Time{}
55+ <-oldSendEntered
56+
57+ replacement := portsmocks.NewMockTransport(t)
58+ replacement.EXPECT().Send(mock.Anything).Return(nil).Maybe()
59+ replacement.EXPECT().Close().Return(nil).Maybe()
60+ attached := make(chan *attachedClient, 1)
61+ go func() {
62+ d.mu.Lock()
63+ attached <- d.finishAttach(sess, replacement, hello.Size, terminalEnv{}, hello)
64+ }()
65+
66+ select {
67+ case ac := <-attached:
68+ require.Same(t, replacement, ac.transport())
69+ case <-time.After(time.Second):
70+ t.Fatal("replacement attach waited for the blocked old render worker")
71+ }
72+
73+ // The exact old link is revoked and closed; it cannot be mistaken for the
74+ // replacement even after the retired worker finally leaves Send.
75+ d.attachmentCleanupWg.Wait()
76+ require.Nil(t, old.transport())
77+ replacement.AssertNotCalled(t, "Close")
78+}
@@ -0,0 +1,78 @@
1+package daemon
2+
3+import (
4+ "io"
5+ "sync"
6+ "testing"
7+ "time"
8+
9+ "github.com/stretchr/testify/mock"
10+ "github.com/stretchr/testify/require"
11+
12+ "github.com/bnema/vev/internal/domain"
13+ "github.com/bnema/vev/internal/ports"
14+ portsmocks "github.com/bnema/vev/internal/ports/mocks"
15+)
16+
17+// TestFinishAttachDoesNotWaitForBlockedOldRenderWorker verifies the replacement
18+// boundary rather than relying on a scheduler delay: an old coordinator worker
19+// is already blocked in Send when finishAttach publishes its replacement.
20+func TestFinishAttachDoesNotWaitForBlockedOldRenderWorker(t *testing.T) {
21+ pty, releasePTY := newBlockingPTY(t)
22+ defer releasePTY()
23+ clock := newCoordinatorMockClock(t, 4)
24+ d := newTestDaemon(t, newFactory(t, pty), clock.clock)
25+
26+ oldTransport := portsmocks.NewMockTransport(t)
27+ oldSendEntered := make(chan struct{})
28+ oldClosed := make(chan struct{})
29+ var closeOnce sync.Once
30+ oldTransport.EXPECT().Send(mock.Anything).RunAndReturn(func(ports.Frame) error {
31+ close(oldSendEntered)
32+ <-oldClosed
33+ return io.EOF
34+ }).Once()
35+ oldTransport.EXPECT().Close().RunAndReturn(func() error {
36+ closeOnce.Do(func() { close(oldClosed) })
37+ return nil
38+ }).Once()
39+
40+ hello := ports.Hello{
41+ Version: ports.ProtocolVersion,
42+ Intent: ports.IntentNew,
43+ Name: "stalled-attachment",
44+ Size: domain.Size{Cols: 80, Rows: 24},
45+ TermEnv: "xterm-256color",
46+ }
47+ sess, old, err := d.route(hello, oldTransport)
48+ require.NoError(t, err)
49+ rc := sess.renderCoordinator()
50+ lease := rc.attachmentLease(old)
51+ require.True(t, rc.markAttachmentReady(lease))
52+
53+ rc.invalidateForAttachment(old, renderInvalidation{class: invalidateUrgent, reset: true, producer: "test"})
54+ awaitCoordinatorScheduledTimer(t, clock).ch <- time.Time{}
55+ <-oldSendEntered
56+
57+ replacement := portsmocks.NewMockTransport(t)
58+ replacement.EXPECT().Send(mock.Anything).Return(nil).Maybe()
59+ replacement.EXPECT().Close().Return(nil).Maybe()
60+ attached := make(chan *attachedClient, 1)
61+ go func() {
62+ d.mu.Lock()
63+ attached <- d.finishAttach(sess, replacement, hello.Size, terminalEnv{}, hello)
64+ }()
65+
66+ select {
67+ case ac := <-attached:
68+ require.Same(t, replacement, ac.transport())
69+ case <-time.After(time.Second):
70+ t.Fatal("replacement attach waited for the blocked old render worker")
71+ }
72+
73+ // The exact old link is revoked and closed; it cannot be mistaken for the
74+ // replacement even after the retired worker finally leaves Send.
75+ d.attachmentCleanupWg.Wait()
76+ require.Nil(t, old.transport())
77+ replacement.AssertNotCalled(t, "Close")
78+}
M internal/usecase/daemon/status_test.go +5 -16
@@ -1381,22 +1381,11 @@ func TestStatusCoalescesCreateSwitchAndResize(t *testing.T) {
13811381 // The queued create, switch, and resize transitions may collapse into one
13821382 // latest-state wake. Advance every retained coordinator deadline
13831383 // until that coalesced output is observable.
1384- var resized ports.Frame
1385- deadline := time.NewTimer(2 * time.Second)
1386- defer deadline.Stop()
1387- for resized.Type != ports.MsgOutput {
1388- select {
1389- case f := <-sends:
1390- if f.Type != ports.MsgOutput {
1391- t.Fatalf("unexpected frame type %d while awaiting resized output", f.Type)
1392- }
1393- resized = f
1394- case timer := <-clock.timers:
1395- timer.ch <- time.Time{}
1396- case <-deadline.C:
1397- t.Fatal("timed out awaiting resized output frame")
1398- }
1399- }
1384+ resized := awaitCoordinatorOutput(
1385+ t, sends, clock.timers,
1386+ "while awaiting resized output",
1387+ "timed out awaiting resized output frame",
1388+ )
14001389
14011390 for _, f := range []ports.Frame{first, resized} {
14021391 out, err := ports.UnmarshalOutput(f.Payload)
@@ -1381,22 +1381,11 @@ func TestStatusCoalescesCreateSwitchAndResize(t *testing.T) {
1381 // The queued create, switch, and resize transitions may collapse into one1381 // The queued create, switch, and resize transitions may collapse into one
1382 // latest-state wake. Advance every retained coordinator deadline1382 // latest-state wake. Advance every retained coordinator deadline
1383 // until that coalesced output is observable.1383 // until that coalesced output is observable.
1384- var resized ports.Frame1384+ resized := awaitCoordinatorOutput(
1385- deadline := time.NewTimer(2 * time.Second)1385+ t, sends, clock.timers,
1386- defer deadline.Stop()1386+ "while awaiting resized output",
1387- for resized.Type != ports.MsgOutput {1387+ "timed out awaiting resized output frame",
1388- select {1388+ )
1389- case f := <-sends:
1390- if f.Type != ports.MsgOutput {
1391- t.Fatalf("unexpected frame type %d while awaiting resized output", f.Type)
1392- }
1393- resized = f
1394- case timer := <-clock.timers:
1395- timer.ch <- time.Time{}
1396- case <-deadline.C:
1397- t.Fatal("timed out awaiting resized output frame")
1398- }
1399- }
1400 1389
1401 for _, f := range []ports.Frame{first, resized} {1390 for _, f := range []ports.Frame{first, resized} {
1402 out, err := ports.UnmarshalOutput(f.Payload)1391 out, err := ports.UnmarshalOutput(f.Payload)
A internal/usecase/daemon/test_wait_helpers_test.go +55 -0
@@ -0,0 +1,55 @@
1+package daemon
2+
3+import (
4+ "testing"
5+ "time"
6+
7+ "github.com/bnema/vev/internal/ports"
8+)
9+
10+const testWaitTimeout = 2 * time.Second
11+
12+func awaitTestValue[T any](t *testing.T, ch <-chan T, failure string) T {
13+ t.Helper()
14+ timer := time.NewTimer(testWaitTimeout)
15+ defer timer.Stop()
16+ select {
17+ case value := <-ch:
18+ return value
19+ case <-timer.C:
20+ t.Fatal(failure)
21+ var zero T
22+ return zero
23+ }
24+}
25+
26+func awaitTestCompletion(t *testing.T, done <-chan struct{}, failure string) {
27+ t.Helper()
28+ awaitTestValue(t, done, failure)
29+}
30+
31+func awaitCoordinatorOutput(
32+ t *testing.T,
33+ sends <-chan ports.Frame,
34+ timers <-chan *coordinatorMockTimer,
35+ frameContext string,
36+ timeoutFailure string,
37+) ports.Frame {
38+ t.Helper()
39+ deadline := time.NewTimer(testWaitTimeout)
40+ defer deadline.Stop()
41+ for {
42+ select {
43+ case frame := <-sends:
44+ if frame.Type != ports.MsgOutput {
45+ t.Fatalf("unexpected frame type %d %s", frame.Type, frameContext)
46+ }
47+ return frame
48+ case timer := <-timers:
49+ timer.ch <- time.Time{}
50+ case <-deadline.C:
51+ t.Fatal(timeoutFailure)
52+ return ports.Frame{}
53+ }
54+ }
55+}
@@ -0,0 +1,55 @@
1+package daemon
2+
3+import (
4+ "testing"
5+ "time"
6+
7+ "github.com/bnema/vev/internal/ports"
8+)
9+
10+const testWaitTimeout = 2 * time.Second
11+
12+func awaitTestValue[T any](t *testing.T, ch <-chan T, failure string) T {
13+ t.Helper()
14+ timer := time.NewTimer(testWaitTimeout)
15+ defer timer.Stop()
16+ select {
17+ case value := <-ch:
18+ return value
19+ case <-timer.C:
20+ t.Fatal(failure)
21+ var zero T
22+ return zero
23+ }
24+}
25+
26+func awaitTestCompletion(t *testing.T, done <-chan struct{}, failure string) {
27+ t.Helper()
28+ awaitTestValue(t, done, failure)
29+}
30+
31+func awaitCoordinatorOutput(
32+ t *testing.T,
33+ sends <-chan ports.Frame,
34+ timers <-chan *coordinatorMockTimer,
35+ frameContext string,
36+ timeoutFailure string,
37+) ports.Frame {
38+ t.Helper()
39+ deadline := time.NewTimer(testWaitTimeout)
40+ defer deadline.Stop()
41+ for {
42+ select {
43+ case frame := <-sends:
44+ if frame.Type != ports.MsgOutput {
45+ t.Fatalf("unexpected frame type %d %s", frame.Type, frameContext)
46+ }
47+ return frame
48+ case timer := <-timers:
49+ timer.ch <- time.Time{}
50+ case <-deadline.C:
51+ t.Fatal(timeoutFailure)
52+ return ports.Frame{}
53+ }
54+ }
55+}
M internal/usecase/daemon/transactional_resize.go +3 -0
@@ -243,6 +243,9 @@ func (d *Daemon) retryResizeMembers(sess *session, ac *attachedClient, lease *at
243243 rc.scheduleResizeRetryForLease(epoch, ac, lease, func() { d.retryResizeMembers(sess, ac, lease, epoch, failed) })
244244 }
245245 if succeeded {
246+ // Retry completion changes VT state after the original layout commit.
247+ // Keep a named session's eventual snapshot generation aligned with it.
248+ markSnapshotDirty(sess)
246249 rc.invalidateForLease(ac, lease, renderInvalidation{class: invalidateUrgent, reset: true, producer: "transactional_resize.go"})
247250 }
248251 }
@@ -243,6 +243,9 @@ func (d *Daemon) retryResizeMembers(sess *session, ac *attachedClient, lease *at
243 rc.scheduleResizeRetryForLease(epoch, ac, lease, func() { d.retryResizeMembers(sess, ac, lease, epoch, failed) })243 rc.scheduleResizeRetryForLease(epoch, ac, lease, func() { d.retryResizeMembers(sess, ac, lease, epoch, failed) })
244 }244 }
245 if succeeded {245 if succeeded {
246+ // Retry completion changes VT state after the original layout commit.
247+ // Keep a named session's eventual snapshot generation aligned with it.
248+ markSnapshotDirty(sess)
246 rc.invalidateForLease(ac, lease, renderInvalidation{class: invalidateUrgent, reset: true, producer: "transactional_resize.go"})249 rc.invalidateForLease(ac, lease, renderInvalidation{class: invalidateUrgent, reset: true, producer: "transactional_resize.go"})
247 }250 }
248 }251 }
M internal/usecase/daemon/transactional_resize_test.go +35 -0
@@ -234,6 +234,41 @@ func TestTransactionalResizePartialFailureCommitsOnlySuccessfulPTYState(t *testi
234234 require.NotEqual(t, oldFailedRect, second.rect, "failed PTY rectangle still follows the committed layout")
235235 }
236236
237+// A retry changes VT state after the committed layout, so it needs a distinct
238+// snapshot generation. A failed retry must retain the prior screen and dirty
239+// generation.
240+func TestTransactionalResizeRetryMarksNamedSnapshotDirtyOnlyOnSuccess(t *testing.T) {
241+ resizeErr := errors.New("scripted resize failure")
242+ for _, tc := range []struct {
243+ name string
244+ errs []error
245+ wantGeneration uint64
246+ wantScreen domain.Size
247+ }{
248+ {name: "success", errs: []error{resizeErr, nil}, wantGeneration: 2, wantScreen: domain.Size{Cols: 100, Rows: 28}},
249+ {name: "failure", errs: []error{resizeErr, resizeErr}, wantGeneration: 1, wantScreen: domain.Size{Cols: 80, Rows: 23}},
250+ } {
251+ t.Run(tc.name, func(t *testing.T) {
252+ pty := &transactionalResizePTY{errs: tc.errs}
253+ d, sess, ac, _ := newManualSessionWithPTYs(t, pty)
254+ sess.snapEligible.Store(true)
255+ tb, p := sess.activeTab(), sess.activeTab().focusedPane()
256+
257+ require.True(t, d.requestTransactionalResize(sess, ac, domain.Size{Cols: 100, Rows: 30}, true))
258+ rc := sess.renderCoordinator()
259+ snapshot := rc.resizeSnapshot()
260+ d.retryResizeMembers(sess, ac, rc.attachmentLease(ac), snapshot.epoch, []resizeMember{{session: sess, tab: tb, pane: p, rect: p.rect}})
261+
262+ sess.snapshotMu.Lock()
263+ generation := sess.snapshotGeneration
264+ sess.snapshotMu.Unlock()
265+ require.Equal(t, tc.wantGeneration, generation)
266+ require.True(t, sess.snapDirty.Load())
267+ require.Equal(t, tc.wantScreen, domain.Size{Cols: p.screen.Frame.Width, Rows: p.screen.Frame.Height})
268+ })
269+ }
270+}
271+
237272 // S3 acceptance: a failed member is retried only for the newest committed
238273 // epoch. An intervening request supersedes the old retry, and a successful
239274 // retry must resize its VT before the forced full-frame emission.
@@ -234,6 +234,41 @@ func TestTransactionalResizePartialFailureCommitsOnlySuccessfulPTYState(t *testi
234 require.NotEqual(t, oldFailedRect, second.rect, "failed PTY rectangle still follows the committed layout")234 require.NotEqual(t, oldFailedRect, second.rect, "failed PTY rectangle still follows the committed layout")
235 }235 }
236 236
237+// A retry changes VT state after the committed layout, so it needs a distinct
238+// snapshot generation. A failed retry must retain the prior screen and dirty
239+// generation.
240+func TestTransactionalResizeRetryMarksNamedSnapshotDirtyOnlyOnSuccess(t *testing.T) {
241+ resizeErr := errors.New("scripted resize failure")
242+ for _, tc := range []struct {
243+ name string
244+ errs []error
245+ wantGeneration uint64
246+ wantScreen domain.Size
247+ }{
248+ {name: "success", errs: []error{resizeErr, nil}, wantGeneration: 2, wantScreen: domain.Size{Cols: 100, Rows: 28}},
249+ {name: "failure", errs: []error{resizeErr, resizeErr}, wantGeneration: 1, wantScreen: domain.Size{Cols: 80, Rows: 23}},
250+ } {
251+ t.Run(tc.name, func(t *testing.T) {
252+ pty := &transactionalResizePTY{errs: tc.errs}
253+ d, sess, ac, _ := newManualSessionWithPTYs(t, pty)
254+ sess.snapEligible.Store(true)
255+ tb, p := sess.activeTab(), sess.activeTab().focusedPane()
256+
257+ require.True(t, d.requestTransactionalResize(sess, ac, domain.Size{Cols: 100, Rows: 30}, true))
258+ rc := sess.renderCoordinator()
259+ snapshot := rc.resizeSnapshot()
260+ d.retryResizeMembers(sess, ac, rc.attachmentLease(ac), snapshot.epoch, []resizeMember{{session: sess, tab: tb, pane: p, rect: p.rect}})
261+
262+ sess.snapshotMu.Lock()
263+ generation := sess.snapshotGeneration
264+ sess.snapshotMu.Unlock()
265+ require.Equal(t, tc.wantGeneration, generation)
266+ require.True(t, sess.snapDirty.Load())
267+ require.Equal(t, tc.wantScreen, domain.Size{Cols: p.screen.Frame.Width, Rows: p.screen.Frame.Height})
268+ })
269+ }
270+}
271+
237 // S3 acceptance: a failed member is retried only for the newest committed272 // S3 acceptance: a failed member is retried only for the newest committed
238 // epoch. An intervening request supersedes the old retry, and a successful273 // epoch. An intervening request supersedes the old retry, and a successful
239 // retry must resize its VT before the forced full-frame emission.274 // retry must resize its VT before the forced full-frame emission.
M internal/usecase/daemon/transport_observability_test.go +4 -4
@@ -25,7 +25,7 @@ func TestTransportObservabilityDaemonBoundaries(t *testing.T) {
2525 if !d.resizeForFirstPaint(sess, ac, domain.Size{Cols: 90, Rows: 25}) {
2626 t.Fatal("first resize was not accepted")
2727 }
28- first := <-sends
28+ first := awaitFrame(t, sends, ports.MsgOutput)
2929 if first.Type != ports.MsgOutput {
3030 t.Fatalf("first frame = %v, want output", first.Type)
3131 }
@@ -38,7 +38,7 @@ func TestTransportObservabilityDaemonBoundaries(t *testing.T) {
3838 }
3939 ac.ackOutputState(output.NewStateNum)
4040 rc.notifyAck()
41- second := <-sends
41+ second := awaitFrame(t, sends, ports.MsgOutput)
4242 if second.Type != ports.MsgOutput {
4343 t.Fatalf("second frame = %v, want output", second.Type)
4444 }
@@ -118,7 +118,7 @@ func TestTransportObservabilityBlockedRenderObserverReleasesArchitectureLocks(t
118118 close(paintDone)
119119 }()
120120 awaitDaemonObserver(t, observer.entered, "blocked render observer")
121- frame := <-sends // The frame was emitted before the observer began its I/O.
121+ frame := awaitFrame(t, sends, ports.MsgOutput) // The frame was emitted before the observer began its I/O.
122122 output, err := ports.UnmarshalOutput(frame.Payload)
123123 if err != nil {
124124 t.Fatalf("decode emitted output: %v", err)
@@ -153,7 +153,7 @@ func TestTransportObservabilityBlockedRenderObserverReleasesArchitectureLocks(t
153153 d.paint(sess, ac, false, nil)
154154 close(secondPaint)
155155 }()
156- <-sends // The second frame is live even though serialized observer I/O waits.
156+ _ = awaitFrame(t, sends, ports.MsgOutput) // The second frame is live even though serialized observer I/O waits.
157157
158158 close(observer.release)
159159 awaitDaemonObserver(t, paintDone, "first render completion")
@@ -25,7 +25,7 @@ func TestTransportObservabilityDaemonBoundaries(t *testing.T) {
25 if !d.resizeForFirstPaint(sess, ac, domain.Size{Cols: 90, Rows: 25}) {25 if !d.resizeForFirstPaint(sess, ac, domain.Size{Cols: 90, Rows: 25}) {
26 t.Fatal("first resize was not accepted")26 t.Fatal("first resize was not accepted")
27 }27 }
28- first := <-sends28+ first := awaitFrame(t, sends, ports.MsgOutput)
29 if first.Type != ports.MsgOutput {29 if first.Type != ports.MsgOutput {
30 t.Fatalf("first frame = %v, want output", first.Type)30 t.Fatalf("first frame = %v, want output", first.Type)
31 }31 }
@@ -38,7 +38,7 @@ func TestTransportObservabilityDaemonBoundaries(t *testing.T) {
38 }38 }
39 ac.ackOutputState(output.NewStateNum)39 ac.ackOutputState(output.NewStateNum)
40 rc.notifyAck()40 rc.notifyAck()
41- second := <-sends41+ second := awaitFrame(t, sends, ports.MsgOutput)
42 if second.Type != ports.MsgOutput {42 if second.Type != ports.MsgOutput {
43 t.Fatalf("second frame = %v, want output", second.Type)43 t.Fatalf("second frame = %v, want output", second.Type)
44 }44 }
@@ -118,7 +118,7 @@ func TestTransportObservabilityBlockedRenderObserverReleasesArchitectureLocks(t
118 close(paintDone)118 close(paintDone)
119 }()119 }()
120 awaitDaemonObserver(t, observer.entered, "blocked render observer")120 awaitDaemonObserver(t, observer.entered, "blocked render observer")
121- frame := <-sends // The frame was emitted before the observer began its I/O.121+ frame := awaitFrame(t, sends, ports.MsgOutput) // The frame was emitted before the observer began its I/O.
122 output, err := ports.UnmarshalOutput(frame.Payload)122 output, err := ports.UnmarshalOutput(frame.Payload)
123 if err != nil {123 if err != nil {
124 t.Fatalf("decode emitted output: %v", err)124 t.Fatalf("decode emitted output: %v", err)
@@ -153,7 +153,7 @@ func TestTransportObservabilityBlockedRenderObserverReleasesArchitectureLocks(t
153 d.paint(sess, ac, false, nil)153 d.paint(sess, ac, false, nil)
154 close(secondPaint)154 close(secondPaint)
155 }()155 }()
156- <-sends // The second frame is live even though serialized observer I/O waits.156+ _ = awaitFrame(t, sends, ports.MsgOutput) // The second frame is live even though serialized observer I/O waits.
157 157
158 close(observer.release)158 close(observer.release)
159 awaitDaemonObserver(t, paintDone, "first render completion")159 awaitDaemonObserver(t, paintDone, "first render completion")
M pkg/vt/history_codec.go +61 -57
@@ -135,53 +135,13 @@ func appendHistoryCell(dst []byte, cell renderer.Cell) []byte {
135135 }
136136
137137 // UnmarshalHistory strictly decodes a MarshalHistory payload. It rejects
138-// malformed declarations, truncated data, and trailing bytes.
138+// malformed declarations, cells, truncated data, and trailing bytes.
139139 func UnmarshalHistory(data []byte) (HistoryView, error) {
140- if len(data) < 9 || string(data[:4]) != historyMagic || data[4] != historyVersion {
141- return HistoryView{}, fmt.Errorf("unmarshal history: %w", errInvalidHistory)
142- }
143- if _, ok := preflightHistory(data[5:]); !ok {
144- return HistoryView{}, fmt.Errorf("unmarshal history: %w", errInvalidHistory)
145- }
146-
147- p := historyParser{data: data[5:]}
148- chunkCount, ok := p.uint32()
149- if !ok || uint64(chunkCount) > uint64(len(p.data))/4 {
150- return HistoryView{}, fmt.Errorf("unmarshal history: %w", errInvalidHistory)
151- }
152- chunks := make([]*HistoryChunk, 0, chunkCount)
153- rowsTotal := 0
154- for range chunkCount {
155- rowCount, ok := p.uint32()
156- if !ok || rowCount == 0 || rowCount > maxHistoryChunkRows || uint64(rowCount) > uint64(len(p.data))/4 {
157- return HistoryView{}, fmt.Errorf("unmarshal history: %w", errInvalidHistory)
158- }
159- if rowsTotal > math.MaxInt-int(rowCount) {
160- return HistoryView{}, fmt.Errorf("unmarshal history: %w", errInvalidHistory)
161- }
162- rows := make([][]renderer.Cell, 0, rowCount)
163- for range rowCount {
164- cellCount, ok := p.uint32()
165- if !ok || uint64(cellCount) > maxHistoryRowCells || uint64(cellCount) > uint64(len(p.data))/historyCellBytes || uint64(cellCount) > uint64(math.MaxInt) {
166- return HistoryView{}, fmt.Errorf("unmarshal history: %w", errInvalidHistory)
167- }
168- row := make([]renderer.Cell, cellCount)
169- for i := range row {
170- cell, ok := p.cell()
171- if !ok || !validHistoryCell(cell) {
172- return HistoryView{}, fmt.Errorf("unmarshal history: %w", errInvalidHistory)
173- }
174- row[i] = cell
175- }
176- rows = append(rows, row)
177- }
178- rowsTotal += int(rowCount)
179- chunks = append(chunks, &HistoryChunk{rows: rows})
180- }
181- if len(p.data) != 0 {
140+ view, _, ok := parseHistory(data, true)
141+ if !ok {
182142 return HistoryView{}, fmt.Errorf("unmarshal history: %w", errInvalidHistory)
183143 }
184- return HistoryView{chunks: chunks, rows: rowsTotal}, nil
144+ return view, nil
185145 }
186146
187147 type historyDecodeStats struct {
@@ -192,35 +152,79 @@ type historyDecodeStats struct {
192152 bytes uint64
193153 }
194154
195-// preflightHistory validates declarations and aggregate budgets without
196-// allocating decoded history. Its totals can be composed by snapshot decoders.
155+// preflightHistory validates a history blob without allocating decoded rows.
156+// It shares parseHistory with UnmarshalHistory so both paths enforce the same
157+// structural, cell, and aggregate-budget constraints.
197158 func preflightHistory(data []byte) (historyDecodeStats, bool) {
198- p := historyParser{data: data}
159+ _, stats, ok := parseHistory(data, false)
160+ return stats, ok
161+}
162+
163+// parseHistory is the authoritative history payload validation path. populate
164+// controls whether validated cells are retained in a HistoryView.
165+func parseHistory(data []byte, populate bool) (HistoryView, historyDecodeStats, bool) {
166+ if len(data) < 9 || string(data[:4]) != historyMagic || data[4] != historyVersion {
167+ return HistoryView{}, historyDecodeStats{}, false
168+ }
169+
170+ p := historyParser{data: data[5:]}
199171 chunkCount, ok := p.uint32()
200172 if !ok || uint64(chunkCount) > maxHistoryChunks || uint64(chunkCount) > uint64(len(p.data))/4 {
201- return historyDecodeStats{}, false
173+ return HistoryView{}, historyDecodeStats{}, false
202174 }
203175 stats := historyDecodeStats{chunks: uint64(chunkCount)}
176+ var chunks []*HistoryChunk
177+ if populate {
178+ chunks = make([]*HistoryChunk, 0, chunkCount)
179+ }
204180 for range chunkCount {
205181 rowCount, ok := p.uint32()
206- if !ok || rowCount == 0 || rowCount > maxHistoryChunkRows || uint64(rowCount) > uint64(len(p.data))/4 || !addHistoryDecodeBudget(&stats.rows, uint64(rowCount), maxHistoryRows) {
207- return historyDecodeStats{}, false
182+ if !ok || rowCount == 0 || rowCount > maxHistoryChunkRows ||
183+ uint64(rowCount) > uint64(len(p.data))/4 ||
184+ !addHistoryDecodeBudget(&stats.rows, uint64(rowCount), maxHistoryRows) {
185+ return HistoryView{}, historyDecodeStats{}, false
186+ }
187+
188+ var rows [][]renderer.Cell
189+ if populate {
190+ rows = make([][]renderer.Cell, 0, rowCount)
208191 }
209192 for range rowCount {
210193 cellCount, ok := p.uint32()
211194 cellBytes, validCellBytes := historyCellByteCount(uint64(cellCount))
212- if !ok || uint64(cellCount) > maxHistoryRowCells || !validCellBytes || uint64(cellCount) > uint64(len(p.data))/historyCellBytes {
213- return historyDecodeStats{}, false
214- }
215- if !addHistoryDecodeBudget(&stats.cells, uint64(cellCount), maxHistoryCells) ||
195+ if !ok || uint64(cellCount) > maxHistoryRowCells || !validCellBytes ||
196+ uint64(cellCount) > uint64(len(p.data))/historyCellBytes ||
197+ !addHistoryDecodeBudget(&stats.cells, uint64(cellCount), maxHistoryCells) ||
216198 !addHistoryDecodeBudget(&stats.styles, uint64(cellCount), maxHistoryDecodeStyles) ||
217199 !addHistoryDecodeBudget(&stats.bytes, cellBytes, maxHistoryDecodedBytes) {
218- return historyDecodeStats{}, false
200+ return HistoryView{}, historyDecodeStats{}, false
219201 }
220- p.data = p.data[int(cellBytes):]
202+
203+ var row []renderer.Cell
204+ if populate {
205+ row = make([]renderer.Cell, cellCount)
206+ }
207+ for i := uint32(0); i < cellCount; i++ {
208+ cell, ok := p.cell()
209+ if !ok || !validHistoryCell(cell) {
210+ return HistoryView{}, historyDecodeStats{}, false
211+ }
212+ if populate {
213+ row[i] = cell
214+ }
215+ }
216+ if populate {
217+ rows = append(rows, row)
218+ }
219+ }
220+ if populate {
221+ chunks = append(chunks, &HistoryChunk{rows: rows})
221222 }
222223 }
223- return stats, len(p.data) == 0
224+ if len(p.data) != 0 {
225+ return HistoryView{}, historyDecodeStats{}, false
226+ }
227+ return HistoryView{chunks: chunks, rows: int(stats.rows)}, stats, true
224228 }
225229
226230 func historyCellByteCount(cellCount uint64) (uint64, bool) {
@@ -135,53 +135,13 @@ func appendHistoryCell(dst []byte, cell renderer.Cell) []byte {
135 }135 }
136 136
137 // UnmarshalHistory strictly decodes a MarshalHistory payload. It rejects137 // UnmarshalHistory strictly decodes a MarshalHistory payload. It rejects
138-// malformed declarations, truncated data, and trailing bytes.138+// malformed declarations, cells, truncated data, and trailing bytes.
139 func UnmarshalHistory(data []byte) (HistoryView, error) {139 func UnmarshalHistory(data []byte) (HistoryView, error) {
140- if len(data) < 9 || string(data[:4]) != historyMagic || data[4] != historyVersion {140+ view, _, ok := parseHistory(data, true)
141- return HistoryView{}, fmt.Errorf("unmarshal history: %w", errInvalidHistory)141+ if !ok {
142- }
143- if _, ok := preflightHistory(data[5:]); !ok {
144- return HistoryView{}, fmt.Errorf("unmarshal history: %w", errInvalidHistory)
145- }
146-
147- p := historyParser{data: data[5:]}
148- chunkCount, ok := p.uint32()
149- if !ok || uint64(chunkCount) > uint64(len(p.data))/4 {
150- return HistoryView{}, fmt.Errorf("unmarshal history: %w", errInvalidHistory)
151- }
152- chunks := make([]*HistoryChunk, 0, chunkCount)
153- rowsTotal := 0
154- for range chunkCount {
155- rowCount, ok := p.uint32()
156- if !ok || rowCount == 0 || rowCount > maxHistoryChunkRows || uint64(rowCount) > uint64(len(p.data))/4 {
157- return HistoryView{}, fmt.Errorf("unmarshal history: %w", errInvalidHistory)
158- }
159- if rowsTotal > math.MaxInt-int(rowCount) {
160- return HistoryView{}, fmt.Errorf("unmarshal history: %w", errInvalidHistory)
161- }
162- rows := make([][]renderer.Cell, 0, rowCount)
163- for range rowCount {
164- cellCount, ok := p.uint32()
165- if !ok || uint64(cellCount) > maxHistoryRowCells || uint64(cellCount) > uint64(len(p.data))/historyCellBytes || uint64(cellCount) > uint64(math.MaxInt) {
166- return HistoryView{}, fmt.Errorf("unmarshal history: %w", errInvalidHistory)
167- }
168- row := make([]renderer.Cell, cellCount)
169- for i := range row {
170- cell, ok := p.cell()
171- if !ok || !validHistoryCell(cell) {
172- return HistoryView{}, fmt.Errorf("unmarshal history: %w", errInvalidHistory)
173- }
174- row[i] = cell
175- }
176- rows = append(rows, row)
177- }
178- rowsTotal += int(rowCount)
179- chunks = append(chunks, &HistoryChunk{rows: rows})
180- }
181- if len(p.data) != 0 {
182 return HistoryView{}, fmt.Errorf("unmarshal history: %w", errInvalidHistory)142 return HistoryView{}, fmt.Errorf("unmarshal history: %w", errInvalidHistory)
183 }143 }
184- return HistoryView{chunks: chunks, rows: rowsTotal}, nil144+ return view, nil
185 }145 }
186 146
187 type historyDecodeStats struct {147 type historyDecodeStats struct {
@@ -192,35 +152,79 @@ type historyDecodeStats struct {
192 bytes uint64152 bytes uint64
193 }153 }
194 154
195-// preflightHistory validates declarations and aggregate budgets without155+// preflightHistory validates a history blob without allocating decoded rows.
196-// allocating decoded history. Its totals can be composed by snapshot decoders.156+// It shares parseHistory with UnmarshalHistory so both paths enforce the same
157+// structural, cell, and aggregate-budget constraints.
197 func preflightHistory(data []byte) (historyDecodeStats, bool) {158 func preflightHistory(data []byte) (historyDecodeStats, bool) {
198- p := historyParser{data: data}159+ _, stats, ok := parseHistory(data, false)
160+ return stats, ok
161+}
162+
163+// parseHistory is the authoritative history payload validation path. populate
164+// controls whether validated cells are retained in a HistoryView.
165+func parseHistory(data []byte, populate bool) (HistoryView, historyDecodeStats, bool) {
166+ if len(data) < 9 || string(data[:4]) != historyMagic || data[4] != historyVersion {
167+ return HistoryView{}, historyDecodeStats{}, false
168+ }
169+
170+ p := historyParser{data: data[5:]}
199 chunkCount, ok := p.uint32()171 chunkCount, ok := p.uint32()
200 if !ok || uint64(chunkCount) > maxHistoryChunks || uint64(chunkCount) > uint64(len(p.data))/4 {172 if !ok || uint64(chunkCount) > maxHistoryChunks || uint64(chunkCount) > uint64(len(p.data))/4 {
201- return historyDecodeStats{}, false173+ return HistoryView{}, historyDecodeStats{}, false
202 }174 }
203 stats := historyDecodeStats{chunks: uint64(chunkCount)}175 stats := historyDecodeStats{chunks: uint64(chunkCount)}
176+ var chunks []*HistoryChunk
177+ if populate {
178+ chunks = make([]*HistoryChunk, 0, chunkCount)
179+ }
204 for range chunkCount {180 for range chunkCount {
205 rowCount, ok := p.uint32()181 rowCount, ok := p.uint32()
206- if !ok || rowCount == 0 || rowCount > maxHistoryChunkRows || uint64(rowCount) > uint64(len(p.data))/4 || !addHistoryDecodeBudget(&stats.rows, uint64(rowCount), maxHistoryRows) {182+ if !ok || rowCount == 0 || rowCount > maxHistoryChunkRows ||
207- return historyDecodeStats{}, false183+ uint64(rowCount) > uint64(len(p.data))/4 ||
184+ !addHistoryDecodeBudget(&stats.rows, uint64(rowCount), maxHistoryRows) {
185+ return HistoryView{}, historyDecodeStats{}, false
186+ }
187+
188+ var rows [][]renderer.Cell
189+ if populate {
190+ rows = make([][]renderer.Cell, 0, rowCount)
208 }191 }
209 for range rowCount {192 for range rowCount {
210 cellCount, ok := p.uint32()193 cellCount, ok := p.uint32()
211 cellBytes, validCellBytes := historyCellByteCount(uint64(cellCount))194 cellBytes, validCellBytes := historyCellByteCount(uint64(cellCount))
212- if !ok || uint64(cellCount) > maxHistoryRowCells || !validCellBytes || uint64(cellCount) > uint64(len(p.data))/historyCellBytes {195+ if !ok || uint64(cellCount) > maxHistoryRowCells || !validCellBytes ||
213- return historyDecodeStats{}, false196+ uint64(cellCount) > uint64(len(p.data))/historyCellBytes ||
214- }197+ !addHistoryDecodeBudget(&stats.cells, uint64(cellCount), maxHistoryCells) ||
215- if !addHistoryDecodeBudget(&stats.cells, uint64(cellCount), maxHistoryCells) ||
216 !addHistoryDecodeBudget(&stats.styles, uint64(cellCount), maxHistoryDecodeStyles) ||198 !addHistoryDecodeBudget(&stats.styles, uint64(cellCount), maxHistoryDecodeStyles) ||
217 !addHistoryDecodeBudget(&stats.bytes, cellBytes, maxHistoryDecodedBytes) {199 !addHistoryDecodeBudget(&stats.bytes, cellBytes, maxHistoryDecodedBytes) {
218- return historyDecodeStats{}, false200+ return HistoryView{}, historyDecodeStats{}, false
219 }201 }
220- p.data = p.data[int(cellBytes):]202+
203+ var row []renderer.Cell
204+ if populate {
205+ row = make([]renderer.Cell, cellCount)
206+ }
207+ for i := uint32(0); i < cellCount; i++ {
208+ cell, ok := p.cell()
209+ if !ok || !validHistoryCell(cell) {
210+ return HistoryView{}, historyDecodeStats{}, false
211+ }
212+ if populate {
213+ row[i] = cell
214+ }
215+ }
216+ if populate {
217+ rows = append(rows, row)
218+ }
219+ }
220+ if populate {
221+ chunks = append(chunks, &HistoryChunk{rows: rows})
221 }222 }
222 }223 }
223- return stats, len(p.data) == 0224+ if len(p.data) != 0 {
225+ return HistoryView{}, historyDecodeStats{}, false
226+ }
227+ return HistoryView{chunks: chunks, rows: int(stats.rows)}, stats, true
224 }228 }
225 229
226 func historyCellByteCount(cellCount uint64) (uint64, bool) {230 func historyCellByteCount(cellCount uint64) (uint64, bool) {
M pkg/vt/history_codec_test.go +55 -0
@@ -227,6 +227,61 @@ func hostileHistoryDeclarations(chunkCount, rowCount int) []byte {
227227 return data
228228 }
229229
230+func TestHistoryPreflightMatchesUnmarshalMalformedInput(t *testing.T) {
231+ valid, err := MarshalHistory(historyViewWithDimensions(1, 1))
232+ if err != nil {
233+ t.Fatalf("marshal valid history: %v", err)
234+ }
235+ invalidRune := append([]byte(nil), valid...)
236+ binary.BigEndian.PutUint32(invalidRune[17:21], ^uint32(0))
237+ invalidUnderlineStyle := append([]byte(nil), valid...)
238+ invalidUnderlineStyle[17+29] = byte(renderer.UnderlineDashed + 1)
239+
240+ tests := []struct {
241+ name string
242+ data []byte
243+ valid bool
244+ }{
245+ {name: "valid", data: valid, valid: true},
246+ {name: "truncated cell", data: valid[:len(valid)-1]},
247+ {name: "trailing byte", data: append(append([]byte(nil), valid...), 0)},
248+ {name: "invalid rune", data: invalidRune},
249+ {name: "invalid underline style", data: invalidUnderlineStyle},
250+ {name: "aggregate row budget", data: hostileHistoryDeclarations(47, maxHistoryChunkRows)},
251+ {name: "zero row count", data: []byte{'V', 'T', 'H', '1', historyVersion, 0, 0, 0, 1, 0, 0, 0, 0}},
252+ }
253+ for _, tt := range tests {
254+ t.Run(tt.name, func(t *testing.T) {
255+ _, preflightErr := PreflightHistoryBlob(tt.data)
256+ _, unmarshalErr := UnmarshalHistory(tt.data)
257+ if (preflightErr == nil) != tt.valid || (unmarshalErr == nil) != tt.valid {
258+ t.Fatalf("preflight error = %v, unmarshal error = %v, want valid = %t", preflightErr, unmarshalErr, tt.valid)
259+ }
260+ })
261+ }
262+}
263+
264+func FuzzHistoryPreflightMatchesUnmarshal(f *testing.F) {
265+ valid, err := MarshalHistory(historyViewWithDimensions(1, 1))
266+ if err != nil {
267+ f.Fatalf("marshal valid history: %v", err)
268+ }
269+ invalidRune := append([]byte(nil), valid...)
270+ binary.BigEndian.PutUint32(invalidRune[17:21], ^uint32(0))
271+ f.Add(valid)
272+ f.Add(invalidRune)
273+ f.Add([]byte("VTH1\x01\x00\x00\x00\x00"))
274+ f.Add([]byte("not a history payload"))
275+
276+ f.Fuzz(func(t *testing.T, data []byte) {
277+ _, preflightErr := PreflightHistoryBlob(data)
278+ _, unmarshalErr := UnmarshalHistory(data)
279+ if (preflightErr == nil) != (unmarshalErr == nil) {
280+ t.Fatalf("preflight error = %v, unmarshal error = %v", preflightErr, unmarshalErr)
281+ }
282+ })
283+}
284+
230285 func assertHistoryRowsEqual(t *testing.T, view HistoryView, want [][]renderer.Cell) {
231286 t.Helper()
232287 if got := view.Len(); got != len(want) {
@@ -227,6 +227,61 @@ func hostileHistoryDeclarations(chunkCount, rowCount int) []byte {
227 return data227 return data
228 }228 }
229 229
230+func TestHistoryPreflightMatchesUnmarshalMalformedInput(t *testing.T) {
231+ valid, err := MarshalHistory(historyViewWithDimensions(1, 1))
232+ if err != nil {
233+ t.Fatalf("marshal valid history: %v", err)
234+ }
235+ invalidRune := append([]byte(nil), valid...)
236+ binary.BigEndian.PutUint32(invalidRune[17:21], ^uint32(0))
237+ invalidUnderlineStyle := append([]byte(nil), valid...)
238+ invalidUnderlineStyle[17+29] = byte(renderer.UnderlineDashed + 1)
239+
240+ tests := []struct {
241+ name string
242+ data []byte
243+ valid bool
244+ }{
245+ {name: "valid", data: valid, valid: true},
246+ {name: "truncated cell", data: valid[:len(valid)-1]},
247+ {name: "trailing byte", data: append(append([]byte(nil), valid...), 0)},
248+ {name: "invalid rune", data: invalidRune},
249+ {name: "invalid underline style", data: invalidUnderlineStyle},
250+ {name: "aggregate row budget", data: hostileHistoryDeclarations(47, maxHistoryChunkRows)},
251+ {name: "zero row count", data: []byte{'V', 'T', 'H', '1', historyVersion, 0, 0, 0, 1, 0, 0, 0, 0}},
252+ }
253+ for _, tt := range tests {
254+ t.Run(tt.name, func(t *testing.T) {
255+ _, preflightErr := PreflightHistoryBlob(tt.data)
256+ _, unmarshalErr := UnmarshalHistory(tt.data)
257+ if (preflightErr == nil) != tt.valid || (unmarshalErr == nil) != tt.valid {
258+ t.Fatalf("preflight error = %v, unmarshal error = %v, want valid = %t", preflightErr, unmarshalErr, tt.valid)
259+ }
260+ })
261+ }
262+}
263+
264+func FuzzHistoryPreflightMatchesUnmarshal(f *testing.F) {
265+ valid, err := MarshalHistory(historyViewWithDimensions(1, 1))
266+ if err != nil {
267+ f.Fatalf("marshal valid history: %v", err)
268+ }
269+ invalidRune := append([]byte(nil), valid...)
270+ binary.BigEndian.PutUint32(invalidRune[17:21], ^uint32(0))
271+ f.Add(valid)
272+ f.Add(invalidRune)
273+ f.Add([]byte("VTH1\x01\x00\x00\x00\x00"))
274+ f.Add([]byte("not a history payload"))
275+
276+ f.Fuzz(func(t *testing.T, data []byte) {
277+ _, preflightErr := PreflightHistoryBlob(data)
278+ _, unmarshalErr := UnmarshalHistory(data)
279+ if (preflightErr == nil) != (unmarshalErr == nil) {
280+ t.Fatalf("preflight error = %v, unmarshal error = %v", preflightErr, unmarshalErr)
281+ }
282+ })
283+}
284+
230 func assertHistoryRowsEqual(t *testing.T, view HistoryView, want [][]renderer.Cell) {285 func assertHistoryRowsEqual(t *testing.T, view HistoryView, want [][]renderer.Cell) {
231 t.Helper()286 t.Helper()
232 if got := view.Len(); got != len(want) {287 if got := view.Len(); got != len(want) {
M pkg/vt/snapshot.go +1 -4
@@ -33,10 +33,7 @@ func (s *DecodeStats) Add(other DecodeStats) bool {
3333 // PreflightHistoryBlob validates one self-contained history blob without
3434 // allocating decoded rows.
3535 func PreflightHistoryBlob(data []byte) (DecodeStats, error) {
36- if len(data) < 9 || string(data[:4]) != historyMagic || data[4] != historyVersion {
37- return DecodeStats{}, fmt.Errorf("preflight history: %w", errInvalidHistory)
38- }
39- stats, ok := preflightHistory(data[5:])
36+ stats, ok := preflightHistory(data)
4037 if !ok {
4138 return DecodeStats{}, fmt.Errorf("preflight history: %w", errInvalidHistory)
4239 }
@@ -33,10 +33,7 @@ func (s *DecodeStats) Add(other DecodeStats) bool {
33 // PreflightHistoryBlob validates one self-contained history blob without33 // PreflightHistoryBlob validates one self-contained history blob without
34 // allocating decoded rows.34 // allocating decoded rows.
35 func PreflightHistoryBlob(data []byte) (DecodeStats, error) {35 func PreflightHistoryBlob(data []byte) (DecodeStats, error) {
36- if len(data) < 9 || string(data[:4]) != historyMagic || data[4] != historyVersion {36+ stats, ok := preflightHistory(data)
37- return DecodeStats{}, fmt.Errorf("preflight history: %w", errInvalidHistory)
38- }
39- stats, ok := preflightHistory(data[5:])
40 if !ok {37 if !ok {
41 return DecodeStats{}, fmt.Errorf("preflight history: %w", errInvalidHistory)38 return DecodeStats{}, fmt.Errorf("preflight history: %w", errInvalidHistory)
42 }39 }

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 12

Tags 11

↑↓ moveenter openesc close