f5b4f9b fix: address final punch list · vev

M docs/performance.md +12 -6


@@ -12,13 +12,19 @@ Demo flood workloads are scripted in `scripts/bench-workloads.sh` for `yes`, `se
12	12
13	13		## Local baseline status
14	14
15		-	Full end-to-end vev-vs-tmux workload baselines were not run for this change because they require an interactive terminal/daemon pair and external tracing tools around both processes. The reproducible methodology is:
	15	+	Full end-to-end vev-vs-tmux workload baselines are deferred/waived for this non-interactive branch because they require an interactive terminal/daemon pair and external tracing tools around both processes. Run them later from an interactive shell with these exact command shapes, substituting only the vev binary path, tmux binary path, terminal size, duration, and sample file path:
16	16
17		-	1. Record host CPU, OS/kernel, terminal emulator, vev commit, tmux version, terminal size, and workload duration.
18		-	2. Start vev daemon with pprof enabled only when collecting profiles.
19		-	3. Run each workload (`yes`, `seq`, `cat`) for the same fixed duration and terminal size under vev and tmux.
20		-	4. Capture client/daemon syscall summaries with `strace -c` (or platform equivalent), bytes written per rendered frame, and heap profiles or `-benchmem` allocation counts.
21		-	5. Store raw command lines and outputs beside this document before comparing.
	17	+	```sh
	18	+	VEV_PPROF_ADDR=127.0.0.1:6060 strace -ff -c -o vev-yes.strace vev --daemon
	19	+	strace -ff -c -o vev-client-yes.strace vev new perf-yes -- sh -lc 'timeout 30s yes'
	20	+	strace -ff -c -o tmux-yes.strace tmux new-session -d -s perf-yes 'timeout 30s yes'
	21	+	strace -ff -c -o vev-client-seq.strace vev new perf-seq -- sh -lc 'timeout 30s sh -c "while :; do seq 1 10000; done"'
	22	+	strace -ff -c -o tmux-seq.strace tmux new-session -d -s perf-seq 'timeout 30s sh -c "while :; do seq 1 10000; done"'
	23	+	strace -ff -c -o vev-client-cat.strace vev new perf-cat -- sh -lc 'timeout 30s sh -c "while :; do cat /path/to/sample.txt; done"'
	24	+	strace -ff -c -o tmux-cat.strace tmux new-session -d -s perf-cat 'timeout 30s sh -c "while :; do cat /path/to/sample.txt; done"'
	25	+	```
	26	+
	27	+	Record host CPU, OS/kernel, terminal emulator, vev commit, tmux version, terminal size, workload duration, raw command lines, syscall summaries, bytes written per rendered frame, and heap profiles or allocation counts before comparing.
22	28
23	29		Microbenchmarks executed locally for this review using `go test ./internal/adapters/ipc -run '^$' -bench=. -benchmem`:
24	30


@@ -12,13 +12,19 @@ Demo flood workloads are scripted in `scripts/bench-workloads.sh` for `yes`, `se
12			12
13		## Local baseline status	13		## Local baseline status
14			14
15	-	Full end-to-end vev-vs-tmux workload baselines were not run for this change because they require an interactive terminal/daemon pair and external tracing tools around both processes. The reproducible methodology is:	15	+	Full end-to-end vev-vs-tmux workload baselines are deferred/waived for this non-interactive branch because they require an interactive terminal/daemon pair and external tracing tools around both processes. Run them later from an interactive shell with these exact command shapes, substituting only the vev binary path, tmux binary path, terminal size, duration, and sample file path:
16			16
17	-	1. Record host CPU, OS/kernel, terminal emulator, vev commit, tmux version, terminal size, and workload duration.	17	+	```sh
18	-	2. Start vev daemon with pprof enabled only when collecting profiles.	18	+	VEV_PPROF_ADDR=127.0.0.1:6060 strace -ff -c -o vev-yes.strace vev --daemon
19	-	3. Run each workload (`yes`, `seq`, `cat`) for the same fixed duration and terminal size under vev and tmux.	19	+	strace -ff -c -o vev-client-yes.strace vev new perf-yes -- sh -lc 'timeout 30s yes'
20	-	4. Capture client/daemon syscall summaries with `strace -c` (or platform equivalent), bytes written per rendered frame, and heap profiles or `-benchmem` allocation counts.	20	+	strace -ff -c -o tmux-yes.strace tmux new-session -d -s perf-yes 'timeout 30s yes'
21	-	5. Store raw command lines and outputs beside this document before comparing.	21	+	strace -ff -c -o vev-client-seq.strace vev new perf-seq -- sh -lc 'timeout 30s sh -c "while :; do seq 1 10000; done"'
			22	+	strace -ff -c -o tmux-seq.strace tmux new-session -d -s perf-seq 'timeout 30s sh -c "while :; do seq 1 10000; done"'
			23	+	strace -ff -c -o vev-client-cat.strace vev new perf-cat -- sh -lc 'timeout 30s sh -c "while :; do cat /path/to/sample.txt; done"'
			24	+	strace -ff -c -o tmux-cat.strace tmux new-session -d -s perf-cat 'timeout 30s sh -c "while :; do cat /path/to/sample.txt; done"'
			25	+	```
			26	+
			27	+	Record host CPU, OS/kernel, terminal emulator, vev commit, tmux version, terminal size, workload duration, raw command lines, syscall summaries, bytes written per rendered frame, and heap profiles or allocation counts before comparing.
22			28
23		Microbenchmarks executed locally for this review using `go test ./internal/adapters/ipc -run '^$' -bench=. -benchmem`:	29		Microbenchmarks executed locally for this review using `go test ./internal/adapters/ipc -run '^$' -bench=. -benchmem`:
24			30

M internal/adapters/sshstdio/transport.go +11 -3


@@ -10,6 +10,7 @@ import (
10	10		"fmt"
11	11		"io"
12	12		"os/exec"
	13	+	"strings"
13	14		"sync"
14	15
15	16		"github.com/bnema/vev/internal/ports"

@@ -96,15 +97,22 @@ type CommandSpec struct {
96	97		Args []string
97	98		}
98	99
99		-	// BuildCommand constructs the ssh subprocess argv without invoking a shell.
	100	+	// BuildCommand constructs the local ssh subprocess argv without invoking a
	101	+	// local shell. OpenSSH sends the remote command as one string for the remote
	102	+	// user's shell to interpret, so every remote argv word is POSIX single-quoted.
100	103		func BuildCommand(target, session string) CommandSpec {
101		-	args := []string{target, "vev", "_stdio"}
	104	+	remote := []string{shellQuote("vev"), shellQuote("_stdio")}
102	105		if session != "" {
103		-	args = append(args, session)
	106	+	remote = append(remote, shellQuote(session))
104	107		}
	108	+	args := []string{"--", target, strings.Join(remote, " ")}
105	109		return CommandSpec{Path: "ssh", Args: args}
106	110		}
107	111
	112	+	func shellQuote(s string) string {
	113	+	return "'" + strings.ReplaceAll(s, "'", "'\\''") + "'"
	114	+	}
	115	+
108	116		// Dial starts ssh target vev _stdio [session] and returns a Transport over the
109	117		// child process' stdio. The subprocess is started with exec.Command argv, never
110	118		// through a shell.

 	"fmt"
 	"io"
 	"os/exec"
+	"strings"
 	"sync"
 	"github.com/bnema/vev/internal/ports"
 	Args []string
 }
-// BuildCommand constructs the ssh subprocess argv without invoking a shell.
+// BuildCommand constructs the local ssh subprocess argv without invoking a
+// local shell. OpenSSH sends the remote command as one string for the remote
+// user's shell to interpret, so every remote argv word is POSIX single-quoted.
 func BuildCommand(target, session string) CommandSpec {
-	args := []string{target, "vev", "_stdio"}
+	remote := []string{shellQuote("vev"), shellQuote("_stdio")}
 	if session != "" {
-		args = append(args, session)
+		remote = append(remote, shellQuote(session))
 	}
+	args := []string{"--", target, strings.Join(remote, " ")}
 	return CommandSpec{Path: "ssh", Args: args}
 }
+func shellQuote(s string) string {
+	return "'" + strings.ReplaceAll(s, "'", "'\\''") + "'"
+}
 // Dial starts ssh target vev _stdio [session] and returns a Transport over the
 // child process' stdio. The subprocess is started with exec.Command argv, never
 // through a shell.

M internal/adapters/sshstdio/transport_test.go +4 -3


@@ -16,9 +16,10 @@ func TestBuildCommandUsesExecArgs(t *testing.T) {
16	16		session string
17	17		want []string
18	18		}{
19		-	{name: "no session", target: "user@example.com", want: []string{"user@example.com", "vev", "_stdio"}},
20		-	{name: "session kept as single arg", target: "user@example.com", session: "work; rm -rf /", want: []string{"user@example.com", "vev", "_stdio", "work; rm -rf /"}},
21		-	{name: "target kept as single arg", target: "user@host; touch /tmp/pwn", session: "work", want: []string{"user@host; touch /tmp/pwn", "vev", "_stdio", "work"}},
	19	+	{name: "no session", target: "user@example.com", want: []string{"--", "user@example.com", "'vev' '_stdio'"}},
	20	+	{name: "session shell metacharacters are quoted in remote command", target: "user@example.com", session: "work; rm -rf /", want: []string{"--", "user@example.com", "'vev' '_stdio' 'work; rm -rf /'"}},
	21	+	{name: "single quote in session is posix escaped", target: "user@example.com", session: "it's fine", want: []string{"--", "user@example.com", "'vev' '_stdio' 'it'\\''s fine'"}},
	22	+	{name: "target kept as single local arg after option terminator", target: "user@host; touch /tmp/pwn", session: "work", want: []string{"--", "user@host; touch /tmp/pwn", "'vev' '_stdio' 'work'"}},
22	23		}
23	24
24	25		for _, tt := range tests {

 		session string
 		want    []string
 	}{
-		{name: "no session", target: "user@example.com", want: []string{"user@example.com", "vev", "_stdio"}},
+		{name: "no session", target: "user@example.com", want: []string{"--", "user@example.com", "'vev' '_stdio'"}},
-		{name: "session kept as single arg", target: "user@example.com", session: "work; rm -rf /", want: []string{"user@example.com", "vev", "_stdio", "work; rm -rf /"}},
+		{name: "session shell metacharacters are quoted in remote command", target: "user@example.com", session: "work; rm -rf /", want: []string{"--", "user@example.com", "'vev' '_stdio' 'work; rm -rf /'"}},
-		{name: "target kept as single arg", target: "user@host; touch /tmp/pwn", session: "work", want: []string{"user@host; touch /tmp/pwn", "vev", "_stdio", "work"}},
+		{name: "single quote in session is posix escaped", target: "user@example.com", session: "it's fine", want: []string{"--", "user@example.com", "'vev' '_stdio' 'it'\\''s fine'"}},
+		{name: "target kept as single local arg after option terminator", target: "user@host; touch /tmp/pwn", session: "work", want: []string{"--", "user@host; touch /tmp/pwn", "'vev' '_stdio' 'work'"}},
 	}
 	for _, tt := range tests {

M internal/usecase/copy/mode.go +11 -1


@@ -7,7 +7,12 @@ import (
7	7		"github.com/bnema/vev/pkg/renderer"
8	8		)
9	9
10		-	const OSC52ChunkSize = 75_000
	10	+	// OSC52MaxPayloadBytes caps clipboard payloads while vev intentionally emits
	11	+	// exactly one OSC 52 sequence. Splitting one clipboard copy across multiple OSC
	12	+	// 52 sequences replaces the clipboard repeatedly in common terminals, corrupting
	13	+	// the intended result, so oversized selections are deferred until a terminal-
	14	+	// specific continuation protocol is supported.
	15	+	const OSC52MaxPayloadBytes = 75_000
11	16
12	17		// Snapshot is the copy-mode document: scrollback followed by the live screen.
13	18		type Snapshot struct {

@@ -170,7 +175,12 @@ func drawCopyStatus(row []renderer.Cell, m *Mode, total int) {
170	175		}
171	176
172	177		// OSC52 encodes text for clipboard transfer as one complete OSC 52 sequence.
	178	+	// Oversized payloads return no sequence rather than emitting corrupting
	179	+	// multi-sequence replacements.
173	180		func OSC52(text string) [][]byte {
	181	+	if len([]byte(text)) > OSC52MaxPayloadBytes {
	182	+	return nil
	183	+	}
174	184		encoded := base64.StdEncoding.EncodeToString([]byte(text))
175	185		return [][]byte{[]byte("\x1b]52;c;" + encoded + "\x07")}
176	186		}

 	"github.com/bnema/vev/pkg/renderer"
 )
-const OSC52ChunkSize = 75_000
+// OSC52MaxPayloadBytes caps clipboard payloads while vev intentionally emits
+// exactly one OSC 52 sequence. Splitting one clipboard copy across multiple OSC
+// 52 sequences replaces the clipboard repeatedly in common terminals, corrupting
+// the intended result, so oversized selections are deferred until a terminal-
+// specific continuation protocol is supported.
+const OSC52MaxPayloadBytes = 75_000
 // Snapshot is the copy-mode document: scrollback followed by the live screen.
 type Snapshot struct {
 }
 // OSC52 encodes text for clipboard transfer as one complete OSC 52 sequence.
+// Oversized payloads return no sequence rather than emitting corrupting
+// multi-sequence replacements.
 func OSC52(text string) [][]byte {
+	if len([]byte(text)) > OSC52MaxPayloadBytes {
+		return nil
+	}
 	encoded := base64.StdEncoding.EncodeToString([]byte(text))
 	return [][]byte{[]byte("\x1b]52;c;" + encoded + "\x07")}
 }

M internal/usecase/copy/mode_test.go +8 -1


@@ -110,7 +110,7 @@ func TestOSC52Base64Encoding(t *testing.T) {
110	110		text string
111	111		}{
112	112		{name: "small payload", text: "hello\nworld"},
113		-	{name: "large payload", text: strings.Repeat("x", OSC52ChunkSize)},
	113	+	{name: "maximum payload", text: strings.Repeat("x", OSC52MaxPayloadBytes)},
114	114		}
115	115		for _, tt := range tests {
116	116		t.Run(tt.name, func(t *testing.T) {

@@ -128,6 +128,13 @@ func TestOSC52Base64Encoding(t *testing.T) {
128	128		}
129	129		}
130	130
	131	+	func TestOSC52OversizedPayloadIsDeferred(t *testing.T) {
	132	+	chunks := OSC52(strings.Repeat("x", OSC52MaxPayloadBytes+1))
	133	+	if len(chunks) != 0 {
	134	+	t.Fatalf("len(chunks) = %d, want 0 to avoid corrupt multi-sequence clipboard replacement", len(chunks))
	135	+	}
	136	+	}
	137	+
131	138		func decodeOSC52Chunk(t *testing.T, chunk []byte) []byte {
132	139		t.Helper()
133	140		s := string(chunk)

 		text string
 	}{
 		{name: "small payload", text: "hello\nworld"},
-		{name: "large payload", text: strings.Repeat("x", OSC52ChunkSize)},
+		{name: "maximum payload", text: strings.Repeat("x", OSC52MaxPayloadBytes)},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 	}
 }
+func TestOSC52OversizedPayloadIsDeferred(t *testing.T) {
+	chunks := OSC52(strings.Repeat("x", OSC52MaxPayloadBytes+1))
+	if len(chunks) != 0 {
+		t.Fatalf("len(chunks) = %d, want 0 to avoid corrupt multi-sequence clipboard replacement", len(chunks))
+	}
+}
 func decodeOSC52Chunk(t *testing.T, chunk []byte) []byte {
 	t.Helper()
 	s := string(chunk)

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


@@ -831,7 +831,9 @@ func (h daemonKeyHandler) Forward(data []byte) {
831	831		if win == nil {
832	832		return
833	833		}
834		-	_, _ = win.pty.Write(data)
	834	+	if _, err := win.pty.Write(data); err != nil {
	835	+	h.d.log.Error("pty write failed", "err", err, "session", h.sess.name)
	836	+	}
835	837		}
836	838
837	839		func (h daemonKeyHandler) Action(action keys.Action) {

 	if win == nil {
 		return
 	}
-	_, _ = win.pty.Write(data)
+	if _, err := win.pty.Write(data); err != nil {
+		h.d.log.Error("pty write failed", "err", err, "session", h.sess.name)
+	}
 }
 func (h daemonKeyHandler) Action(action keys.Action) {

M internal/usecase/daemon/daemon_test.go +20 -0

@@ -1,7 +1,9 @@
1	1		package daemon
2	2
3	3		import (
	4	+	"bytes"
4	5		"context"
	6	+	"errors"
5	7		"io"
6	8		"log/slog"
7	9		"strings"

@@ -229,6 +231,24 @@ func newManualSessionWithPTYs(t testing.T, ptys ...ports.PTY) (Daemon, *sessio
229	231		return d, sess, ac, sends
230	232		}
231	233
	234	+	func TestPTYWriteErrorIsLogged(t *testing.T) {
	235	+	var logs bytes.Buffer
	236	+	d := New(nil, stubClock{}, slog.New(slog.NewTextHandler(&logs, nil)))
	237	+	errBoom := errors.New("boom")
	238	+	p := portsmocks.NewMockPTY(t)
	239	+	p.EXPECT().Write([]byte("input")).Return(0, errBoom).Once()
	240	+	win := &window{pty: p, screen: vt.NewScreen(80, 23), dirty: make(chan struct{}, 1), size: domain.Size{Cols: 80, Rows: 23}}
	241	+	sess := &session{id: "manual", name: "work", windows: []*window{win}}
	242	+	ac := &attachedClient{}
	243	+
	244	+	daemonKeyHandler{d: d, sess: sess, ac: ac}.Forward([]byte("input"))
	245	+
	246	+	got := logs.String()
	247	+	if !strings.Contains(got, "pty write failed") \|\| !strings.Contains(got, "boom") \|\| !strings.Contains(got, "work") {
	248	+	t.Fatalf("log output %q does not contain PTY write failure details", got)
	249	+	}
	250	+	}
	251	+
232	252		func newManualWindowSession(t testing.T, n int) (Daemon, session, attachedClient, chan ports.Frame, []func()) {
233	253		t.Helper()
234	254		ptys := make([]ports.PTY, 0, n)

@@ -1,7 +1,9 @@
1	package daemon	1		package daemon
2		2
3	import (	3		import (
		4	+	"bytes"
4	"context"	5		"context"
		6	+	"errors"
5	"io"	7		"io"
6	"log/slog"	8		"log/slog"
7	"strings"	9		"strings"

@@ -229,6 +231,24 @@ func newManualSessionWithPTYs(t testing.T, ptys ...ports.PTY) (Daemon, *sessio
229	return d, sess, ac, sends	231		return d, sess, ac, sends
230	}	232		}
231		233
		234	+	func TestPTYWriteErrorIsLogged(t *testing.T) {
		235	+	var logs bytes.Buffer
		236	+	d := New(nil, stubClock{}, slog.New(slog.NewTextHandler(&logs, nil)))
		237	+	errBoom := errors.New("boom")
		238	+	p := portsmocks.NewMockPTY(t)
		239	+	p.EXPECT().Write([]byte("input")).Return(0, errBoom).Once()
		240	+	win := &window{pty: p, screen: vt.NewScreen(80, 23), dirty: make(chan struct{}, 1), size: domain.Size{Cols: 80, Rows: 23}}
		241	+	sess := &session{id: "manual", name: "work", windows: []*window{win}}
		242	+	ac := &attachedClient{}
		243	+
		244	+	daemonKeyHandler{d: d, sess: sess, ac: ac}.Forward([]byte("input"))
		245	+
		246	+	got := logs.String()
		247	+	if !strings.Contains(got, "pty write failed") \|\| !strings.Contains(got, "boom") \|\| !strings.Contains(got, "work") {
		248	+	t.Fatalf("log output %q does not contain PTY write failure details", got)
		249	+	}
		250	+	}
		251	+
232	func newManualWindowSession(t testing.T, n int) (Daemon, session, attachedClient, chan ports.Frame, []func()) {	252		func newManualWindowSession(t testing.T, n int) (Daemon, session, attachedClient, chan ports.Frame, []func()) {
233	t.Helper()	253		t.Helper()
234	ptys := make([]ports.PTY, 0, n)	254		ptys := make([]ports.PTY, 0, n)