4276a75 fix(capture): address recording and allocation review findings · nefercap

M internal/adapters/cli/cli.go +2 -2


@@ -25,8 +25,8 @@ const (
25	25		Rec Command = "rec"
26	26		Stop Command = "stop"
27	27		Status Command = "status"
28		-	// Outputs, Screenshot and Record are the scriptable commands. They never
29		-	// open the selector and require -file.
	28	+	// Outputs, Screenshot and Record never open the selector. Outputs needs
	29	+	// no -file; Screenshot requires it unless clipboard-only, Record always.
30	30		Outputs Command = "outputs"
31	31		Screenshot Command = "screenshot"
32	32		Record Command = "record"


@@ -25,8 +25,8 @@ const (
25		Rec Command = "rec"	25		Rec Command = "rec"
26		Stop Command = "stop"	26		Stop Command = "stop"
27		Status Command = "status"	27		Status Command = "status"
28	-	// Outputs, Screenshot and Record are the scriptable commands. They never	28	+	// Outputs, Screenshot and Record never open the selector. Outputs needs
29	-	// open the selector and require -file.	29	+	// no -file; Screenshot requires it unless clipboard-only, Record always.
30		Outputs Command = "outputs"	30		Outputs Command = "outputs"
31		Screenshot Command = "screenshot"	31		Screenshot Command = "screenshot"
32		Record Command = "record"	32		Record Command = "record"

M internal/adapters/ffmpeg/ffmpeg.go +5 -0


@@ -90,6 +90,9 @@ type Writer struct {
90	90
91	91		watchMu sync.Mutex
92	92		watchDead bool
	93	+	// Saved reports whether Close retained the reserved file. Read it only
	94	+	// after the owning Close or Abort returns, independently of capture errors.
	95	+	Saved bool
93	96		startStop func() bool
94	97		writeStop func() bool
95	98		writeDone <-chan struct{}

@@ -471,6 +474,8 @@ func (w *Writer) shutdown(abort bool) error {
471	474		}
472	475		if err != nil {
473	476		err = errors.Join(err, w.removeOutput())
	477	+	} else {
	478	+	w.Saved = true
474	479		}
475	480		return err
476	481		}


@@ -90,6 +90,9 @@ type Writer struct {
90		90
91	watchMu sync.Mutex	91		watchMu sync.Mutex
92	watchDead bool	92		watchDead bool
		93	+	// Saved reports whether Close retained the reserved file. Read it only
		94	+	// after the owning Close or Abort returns, independently of capture errors.
		95	+	Saved bool
93	startStop func() bool	96		startStop func() bool
94	writeStop func() bool	97		writeStop func() bool
95	writeDone <-chan struct{}	98		writeDone <-chan struct{}

@@ -471,6 +474,8 @@ func (w *Writer) shutdown(abort bool) error {
471	}	474		}
472	if err != nil {	475		if err != nil {
473	err = errors.Join(err, w.removeOutput())	476		err = errors.Join(err, w.removeOutput())
		477	+	} else {
		478	+	w.Saved = true
474	}	479		}
475	return err	480		return err
476	}	481		}

M internal/adapters/ffmpeg/ffmpeg_test.go +9 -0


@@ -187,6 +187,9 @@ func TestStartRefusesExistingPath(t *testing.T) {
187	187		if got, _ := os.ReadFile(path); string(got) != "keep" {
188	188		t.Fatalf("Close touched foreign file: %q", got)
189	189		}
	190	+	if w.Saved {
	191	+	t.Fatal("reported a pre-existing file as saved")
	192	+	}
190	193		}
191	194
192	195		func TestStartFailureRemovesReservedPath(t *testing.T) {

@@ -260,6 +263,9 @@ func TestDrainLifecycle(t *testing.T) {
260	263		if err := w.Abort(); err != nil {
261	264		t.Fatalf("Abort after Close: %v", err)
262	265		}
	266	+	if !w.Saved {
	267	+	t.Fatal("finalized recording not reported as saved")
	268	+	}
263	269		if err := w.Write(ctx, first); !errors.Is(err, ports.ErrClosed) {
264	270		t.Fatalf("Write after Close: %v", err)
265	271		}

@@ -615,6 +621,9 @@ func TestAbortDiscardsHealthyRecording(t *testing.T) {
615	621		if err := w.Close(); err != nil {
616	622		t.Fatalf("Close after Abort = %v", err)
617	623		}
	624	+	if w.Saved {
	625	+	t.Fatal("aborted recording reported as saved")
	626	+	}
618	627		}
619	628
620	629		func TestTerminalCleanupSparesReplacementFile(t *testing.T) {


@@ -187,6 +187,9 @@ func TestStartRefusesExistingPath(t *testing.T) {
187	if got, _ := os.ReadFile(path); string(got) != "keep" {	187		if got, _ := os.ReadFile(path); string(got) != "keep" {
188	t.Fatalf("Close touched foreign file: %q", got)	188		t.Fatalf("Close touched foreign file: %q", got)
189	}	189		}
		190	+	if w.Saved {
		191	+	t.Fatal("reported a pre-existing file as saved")
		192	+	}
190	}	193		}
191		194
192	func TestStartFailureRemovesReservedPath(t *testing.T) {	195		func TestStartFailureRemovesReservedPath(t *testing.T) {

@@ -260,6 +263,9 @@ func TestDrainLifecycle(t *testing.T) {
260	if err := w.Abort(); err != nil {	263		if err := w.Abort(); err != nil {
261	t.Fatalf("Abort after Close: %v", err)	264		t.Fatalf("Abort after Close: %v", err)
262	}	265		}
		266	+	if !w.Saved {
		267	+	t.Fatal("finalized recording not reported as saved")
		268	+	}
263	if err := w.Write(ctx, first); !errors.Is(err, ports.ErrClosed) {	269		if err := w.Write(ctx, first); !errors.Is(err, ports.ErrClosed) {
264	t.Fatalf("Write after Close: %v", err)	270		t.Fatalf("Write after Close: %v", err)
265	}	271		}

@@ -615,6 +621,9 @@ func TestAbortDiscardsHealthyRecording(t *testing.T) {
615	if err := w.Close(); err != nil {	621		if err := w.Close(); err != nil {
616	t.Fatalf("Close after Abort = %v", err)	622		t.Fatalf("Close after Abort = %v", err)
617	}	623		}
		624	+	if w.Saved {
		625	+	t.Fatal("aborted recording reported as saved")
		626	+	}
618	}	627		}
619		628
620	func TestTerminalCleanupSparesReplacementFile(t *testing.T) {	629		func TestTerminalCleanupSparesReplacementFile(t *testing.T) {

M internal/adapters/indicator/hud.go +18 -6


@@ -4,6 +4,7 @@ import (
4	4		"strconv"
5	5		"time"
6	6		"unicode"
	7	+	"unicode/utf8"
7	8		)
8	9
9	10		// prefix starts the recording label; the elapsed time follows it.

@@ -21,19 +22,30 @@ const maxLabelRunes = 12
21	22		// paragraph separators U+2028 and U+2029, and the bidirectional controls
22	23		// (Unicode Bidi_Control) that could reorder the timer beside it.
23	24		func suffixFor(label string) string {
24		-	var r []rune
	25	+	var r [maxLabelRunes + 1]rune
	26	+	n := 0
25	27		for _, c := range label {
26	28		if keepRune(c) {
27		-	r = append(r, c)
	29	+	r[n] = c
	30	+	n++
	31	+	if n > maxLabelRunes {
	32	+	r[maxLabelRunes-1] = '…'
	33	+	n = maxLabelRunes
	34	+	break
	35	+	}
28	36		}
29	37		}
30		-	if len(r) == 0 {
	38	+	if n == 0 {
31	39		return ""
32	40		}
33		-	if len(r) > maxLabelRunes {
34		-	r = append(r[:maxLabelRunes-1], '…')
	41	+	// Encode prefix and label together: a long rune-to-string conversion
	42	+	// followed by concatenation would allocate an intermediate string too.
	43	+	var buf [len(" · ") + maxLabelRunes*utf8.UTFMax]byte
	44	+	b := buf[:copy(buf[:], " · ")]
	45	+	for _, c := range r[:n] {
	46	+	b = utf8.AppendRune(b, c)
35	47		}
36		-	return " · " + string(r)
	48	+	return string(b)
37	49		}
38	50
39	51		// keepRune reports whether c may appear in the label.

 	"strconv"
 	"time"
 	"unicode"
+	"unicode/utf8"
 )
 // prefix starts the recording label; the elapsed time follows it.
 // paragraph separators U+2028 and U+2029, and the bidirectional controls
 // (Unicode Bidi_Control) that could reorder the timer beside it.
 func suffixFor(label string) string {
-	var r []rune
+	var r [maxLabelRunes + 1]rune
+	n := 0
 	for _, c := range label {
 		if keepRune(c) {
-			r = append(r, c)
+			r[n] = c
+			n++
+			if n > maxLabelRunes {
+				r[maxLabelRunes-1] = '…'
+				n = maxLabelRunes
+				break
+			}
 		}
 	}
-	if len(r) == 0 {
+	if n == 0 {
 		return ""
 	}
-	if len(r) > maxLabelRunes {
+	// Encode prefix and label together: a long rune-to-string conversion
-		r = append(r[:maxLabelRunes-1], '…')
+	// followed by concatenation would allocate an intermediate string too.
+	var buf [len(" · ") + maxLabelRunes*utf8.UTFMax]byte
+	b := buf[:copy(buf[:], " · ")]
+	for _, c := range r[:n] {
+		b = utf8.AppendRune(b, c)
 	}
-	return " · " + string(r)
+	return string(b)
 }
 // keepRune reports whether c may appear in the label.

M internal/adapters/indicator/indicator_test.go +32 -0


@@ -2,6 +2,7 @@ package indicator
2	2
3	3		import (
4	4		"context"
	5	+	"strings"
5	6		"testing"
6	7		"time"
7	8

@@ -70,6 +71,37 @@ func TestTargetLabelIsSingleLinePrintable(t *testing.T) {
70	71		assert.Equal(t, "", suffixFor("\u202e\u2028"))
71	72		}
72	73
	74	+	func TestSuffixLongInput(t *testing.T) {
	75	+	label := strings.Repeat("\x00界", 100000)
	76	+	assert.Equal(t, " · 界界界界界界界界界界界…", suffixFor(label))
	77	+	assert.Equal(t, " · 123456789012", suffixFor("123456789012"+strings.Repeat("\n", 100000)))
	78	+	}
	79	+
	80	+	var suffixSink string
	81	+
	82	+	func TestSuffixAllocations(t *testing.T) {
	83	+	label := strings.Repeat("\x00界", 100000)
	84	+	allocs := testing.AllocsPerRun(100, func() { suffixSink = suffixFor(label) })
	85	+	t.Logf("long suffix: %.0f allocs/run", allocs)
	86	+	if allocs > 1 {
	87	+	t.Fatalf("suffix allocated %.0f times, want only the returned string", allocs)
	88	+	}
	89	+	}
	90	+
	91	+	func BenchmarkSuffixFor(b *testing.B) {
	92	+	for name, label := range map[string]string{
	93	+	"short": "workspace",
	94	+	"long": strings.Repeat("\x00界", 100000),
	95	+	} {
	96	+	b.Run(name, func(b *testing.B) {
	97	+	b.ReportAllocs()
	98	+	for b.Loop() {
	99	+	suffixSink = suffixFor(label)
	100	+	}
	101	+	})
	102	+	}
	103	+	}
	104	+
73	105		func TestRefreshAllocations(t *testing.T) {
74	106		if raceEnabled {
75	107		t.Skip("the race detector adds allocations")


@@ -2,6 +2,7 @@ package indicator
2		2
3	import (	3		import (
4	"context"	4		"context"
		5	+	"strings"
5	"testing"	6		"testing"
6	"time"	7		"time"
7		8

@@ -70,6 +71,37 @@ func TestTargetLabelIsSingleLinePrintable(t *testing.T) {
70	assert.Equal(t, "", suffixFor("\u202e\u2028"))	71		assert.Equal(t, "", suffixFor("\u202e\u2028"))
71	}	72		}
72		73
		74	+	func TestSuffixLongInput(t *testing.T) {
		75	+	label := strings.Repeat("\x00界", 100000)
		76	+	assert.Equal(t, " · 界界界界界界界界界界界…", suffixFor(label))
		77	+	assert.Equal(t, " · 123456789012", suffixFor("123456789012"+strings.Repeat("\n", 100000)))
		78	+	}
		79	+
		80	+	var suffixSink string
		81	+
		82	+	func TestSuffixAllocations(t *testing.T) {
		83	+	label := strings.Repeat("\x00界", 100000)
		84	+	allocs := testing.AllocsPerRun(100, func() { suffixSink = suffixFor(label) })
		85	+	t.Logf("long suffix: %.0f allocs/run", allocs)
		86	+	if allocs > 1 {
		87	+	t.Fatalf("suffix allocated %.0f times, want only the returned string", allocs)
		88	+	}
		89	+	}
		90	+
		91	+	func BenchmarkSuffixFor(b *testing.B) {
		92	+	for name, label := range map[string]string{
		93	+	"short": "workspace",
		94	+	"long": strings.Repeat("\x00界", 100000),
		95	+	} {
		96	+	b.Run(name, func(b *testing.B) {
		97	+	b.ReportAllocs()
		98	+	for b.Loop() {
		99	+	suffixSink = suffixFor(label)
		100	+	}
		101	+	})
		102	+	}
		103	+	}
		104	+
73	func TestRefreshAllocations(t *testing.T) {	105		func TestRefreshAllocations(t *testing.T) {
74	if raceEnabled {	106		if raceEnabled {
75	t.Skip("the race detector adds allocations")	107		t.Skip("the race detector adds allocations")

M internal/adapters/selection/model.go +2 -1


@@ -235,7 +235,7 @@ func outputKey(name string, n int) (int, bool) {
235	235		return i, i < n
236	236		}
237	237
238		-	// settle mirrors the picker's terminal state and refreshes the label.
	238	+	// refreshFooter rebuilds the footer only when the target kind or workspace changes.
239	239		func (m *model) refreshFooter() {
240	240		kind := m.picker.Kind()
241	241		_, _, workspace := m.picker.Workspace()

@@ -245,6 +245,7 @@ func (m *model) refreshFooter() {
245	245		}
246	246		}
247	247
	248	+	// settle mirrors the picker's terminal state and refreshes the label.
248	249		func (m *model) settle() {
249	250		m.refreshFooter()
250	251		switch m.picker.Status() {

 	return i, i < n
 }
-// settle mirrors the picker's terminal state and refreshes the label.
+// refreshFooter rebuilds the footer only when the target kind or workspace changes.
 func (m *model) refreshFooter() {
 	kind := m.picker.Kind()
 	_, _, workspace := m.picker.Workspace()
 	}
 }
+// settle mirrors the picker's terminal state and refreshes the label.
 func (m *model) settle() {
 	m.refreshFooter()
 	switch m.picker.Status() {

M internal/app/app.go +1 -2


@@ -10,7 +10,6 @@ import (
10	10
11	11		"github.com/bnema/nefercap/internal/adapters/cli"
12	12		"github.com/bnema/nefercap/internal/adapters/clipboard"
13		-	"github.com/bnema/nefercap/internal/adapters/ffmpeg"
14	13		"github.com/bnema/nefercap/internal/adapters/png"
15	14		"github.com/bnema/nefercap/internal/adapters/wayland"
16	15		"github.com/bnema/nefercap/internal/core"

@@ -95,7 +94,7 @@ func Run(ctx context.Context, options cli.Options, output io.Writer) (err error)
95	94		copied = clipboard.NewWriter(writer, clipboard.New())
96	95		writer = copied
97	96		}
98		-	err = core.New(source, writer, ffmpeg.New()).Run(ctx, selection)
	97	+	err = core.New(source, writer, nil).Run(ctx, selection)
99	98		saved = err == nil && selection.Path != ""
100	99		if copied != nil {
101	100		saved = copied.Saved

 	"github.com/bnema/nefercap/internal/adapters/cli"
 	"github.com/bnema/nefercap/internal/adapters/clipboard"
-	"github.com/bnema/nefercap/internal/adapters/ffmpeg"
 	"github.com/bnema/nefercap/internal/adapters/png"
 	"github.com/bnema/nefercap/internal/adapters/wayland"
 	"github.com/bnema/nefercap/internal/core"
 			copied = clipboard.NewWriter(writer, clipboard.New())
 			writer = copied
 		}
-		err = core.New(source, writer, ffmpeg.New()).Run(ctx, selection)
+		err = core.New(source, writer, nil).Run(ctx, selection)
 		saved = err == nil && selection.Path != ""
 		if copied != nil {
 			saved = copied.Saved

M internal/app/app_test.go +15 -0


@@ -5,9 +5,24 @@ import (
5	5		"errors"
6	6		"testing"
7	7
	8	+	"github.com/bnema/nefercap/internal/core"
	9	+	portsmocks "github.com/bnema/nefercap/internal/mocks/ports"
8	10		"github.com/bnema/nefercap/internal/ports"
9	11		)
10	12
	13	+	func TestScreenshotWithoutVideoWriter(t *testing.T) {
	14	+	ctx := context.Background()
	15	+	target := ports.Target{OutputID: 1}
	16	+	frame := ports.Frame{Pixels: make([]byte, 16), Width: 2, Height: 2, Stride: 8, Format: ports.XRGB8888}
	17	+	source := portsmocks.NewMockSource(t)
	18	+	writer := portsmocks.NewMockScreenshotWriter(t)
	19	+	source.EXPECT().Capture(ctx, target).Return(frame, nil).Once()
	20	+	writer.EXPECT().Save(ctx, frame, "shot.png").Return(nil).Once()
	21	+	if err := core.New(source, writer, nil).Run(ctx, ports.Selection{Mode: ports.Screenshot, Target: target, Path: "shot.png"}); err != nil {
	22	+	t.Fatal(err)
	23	+	}
	24	+	}
	25	+
11	26		func TestDiscoveryError(t *testing.T) {
12	27		ctx, cancel := context.WithCancel(context.Background())
13	28		defer cancel()


@@ -5,9 +5,24 @@ import (
5	"errors"	5		"errors"
6	"testing"	6		"testing"
7		7
		8	+	"github.com/bnema/nefercap/internal/core"
		9	+	portsmocks "github.com/bnema/nefercap/internal/mocks/ports"
8	"github.com/bnema/nefercap/internal/ports"	10		"github.com/bnema/nefercap/internal/ports"
9	)	11		)
10		12
		13	+	func TestScreenshotWithoutVideoWriter(t *testing.T) {
		14	+	ctx := context.Background()
		15	+	target := ports.Target{OutputID: 1}
		16	+	frame := ports.Frame{Pixels: make([]byte, 16), Width: 2, Height: 2, Stride: 8, Format: ports.XRGB8888}
		17	+	source := portsmocks.NewMockSource(t)
		18	+	writer := portsmocks.NewMockScreenshotWriter(t)
		19	+	source.EXPECT().Capture(ctx, target).Return(frame, nil).Once()
		20	+	writer.EXPECT().Save(ctx, frame, "shot.png").Return(nil).Once()
		21	+	if err := core.New(source, writer, nil).Run(ctx, ports.Selection{Mode: ports.Screenshot, Target: target, Path: "shot.png"}); err != nil {
		22	+	t.Fatal(err)
		23	+	}
		24	+	}
		25	+
11	func TestDiscoveryError(t *testing.T) {	26		func TestDiscoveryError(t *testing.T) {
12	ctx, cancel := context.WithCancel(context.Background())	27		ctx, cancel := context.WithCancel(context.Background())
13	defer cancel()	28		defer cancel()

M internal/app/record_interactive.go +7 -6


@@ -56,14 +56,14 @@ func recordInteractive(ctx context.Context, source *wayland.Source, outputs []po
56	56
57	57		hud := startHUD(hudCtx, output, session.Token, hudStop, life)
58	58		end := ending{}
59		-	started := false
	59	+	var video *ffmpeg.Writer
60	60		// Do not start the encoder until the compositor acknowledges exclusion.
61	61		// A protocol roundtrip alone does not prove the core applied the marking.
62	62		select {
63	63		case <-hud.ready:
64		-	started = true
	64	+	video = ffmpeg.New()
65	65		captureDone := make(chan error, 1)
66		-	go func() { captureDone <- core.New(source, nil, ffmpeg.New()).Run(recordCtx, sel) }()
	66	+	go func() { captureDone <- core.New(source, nil, video).Run(recordCtx, sel) }()
67	67		select {
68	68		case end.runErr = <-captureDone:
69	69		life.beginShutdown()

@@ -84,9 +84,10 @@ func recordInteractive(ctx context.Context, source *wayland.Source, outputs []po
84	84		end.parentErr = ctx.Err()
85	85		end.userStop = context.Cause(recordCtx) == errUserStop
86	86		err = classify(end)
87		-	if started && end.runErr == nil {
88		-	// Frames were written: the file exists whatever the HUD reported after.
89		-	outcome.Saved = savedFile(sel.Path)
	87	+	if video != nil {
	88	+	// Finalization may retain frames despite a late capture or HUD error.
	89	+	// The writer knows whether it kept its file, not a pre-existing path.
	90	+	outcome.Saved = video.Saved
90	91		}
91	92		return outcome, err
92	93		}

 	hud := startHUD(hudCtx, output, session.Token, hudStop, life)
 	end := ending{}
-	started := false
+	var video *ffmpeg.Writer
 	// Do not start the encoder until the compositor acknowledges exclusion.
 	// A protocol roundtrip alone does not prove the core applied the marking.
 	select {
 	case <-hud.ready:
-		started = true
+		video = ffmpeg.New()
 		captureDone := make(chan error, 1)
-		go func() { captureDone <- core.New(source, nil, ffmpeg.New()).Run(recordCtx, sel) }()
+		go func() { captureDone <- core.New(source, nil, video).Run(recordCtx, sel) }()
 		select {
 		case end.runErr = <-captureDone:
 			life.beginShutdown()
 	end.parentErr = ctx.Err()
 	end.userStop = context.Cause(recordCtx) == errUserStop
 	err = classify(end)
-	if started && end.runErr == nil {
+	if video != nil {
-		// Frames were written: the file exists whatever the HUD reported after.
+		// Finalization may retain frames despite a late capture or HUD error.
-		outcome.Saved = savedFile(sel.Path)
+		// The writer knows whether it kept its file, not a pre-existing path.
+		outcome.Saved = video.Saved
 	}
 	return outcome, err
 }

M internal/app/record_lifecycle.go +0 -9


@@ -3,7 +3,6 @@ package app
3	3		import (
4	4		"context"
5	5		"errors"
6		-	"os"
7	6		"sync/atomic"
8	7
9	8		"github.com/bnema/nefercap/internal/adapters/uierrors"

@@ -165,11 +164,3 @@ func classify(e ending) error {
165	164		}
166	165		return e.runErr
167	166		}
168		-
169		-	// savedFile reports whether path is a non-empty regular file. Call it only
170		-	// after the core reported success: the writer reserves the path exclusively
171		-	// and removes it when aborting, so a file there is this recording.
172		-	func savedFile(path string) bool {
173		-	info, err := os.Stat(path)
174		-	return err == nil && info.Mode().IsRegular() && info.Size() > 0
175		-	}


@@ -3,7 +3,6 @@ package app
3		import (	3	import (
4		"context"	4	"context"
5		"errors"	5	"errors"
6	-	"os"
7		"sync/atomic"	6	"sync/atomic"
8			7
9		"github.com/bnema/nefercap/internal/adapters/uierrors"	8	"github.com/bnema/nefercap/internal/adapters/uierrors"

@@ -165,11 +164,3 @@ func classify(e ending) error {
165		}	164	}
166		return e.runErr	165	return e.runErr
167		}	166	}
168	-
169	-	// savedFile reports whether path is a non-empty regular file. Call it only
170	-	// after the core reported success: the writer reserves the path exclusively
171	-	// and removes it when aborting, so a file there is this recording.
172	-	func savedFile(path string) bool {
173	-	info, err := os.Stat(path)
174	-	return err == nil && info.Mode().IsRegular() && info.Size() > 0
175	-	}

M internal/app/record_lifecycle_test.go +2 -18


@@ -257,22 +257,6 @@ func TestFinishCapture(t *testing.T) {
257	257		}
258	258		}
259	259
260		-	func TestSavedFile(t *testing.T) {
261		-	dir := t.TempDir()
262		-	missing := filepath.Join(dir, "missing.mp4")
263		-	empty := filepath.Join(dir, "empty.mp4")
264		-	full := filepath.Join(dir, "full.mp4")
265		-	if err := os.WriteFile(empty, nil, 0o600); err != nil {
266		-	t.Fatal(err)
267		-	}
268		-	if err := os.WriteFile(full, []byte("x"), 0o600); err != nil {
269		-	t.Fatal(err)
270		-	}
271		-	if savedFile(missing) \|\| savedFile(empty) \|\| savedFile(dir) \|\| !savedFile(full) {
272		-	t.Fatal("saved file detection wrong")
273		-	}
274		-	}
275		-
276	260		// The production core with generated ports: cancelling during the first
277	261		// capture aborts nothing and writes nothing, and the lifecycle calls it success.
278	262		func TestCancelDuringFirstFrameIsNotAFailure(t *testing.T) {

@@ -293,8 +277,8 @@ func TestCancelDuringFirstFrameIsNotAFailure(t *testing.T) {
293	277		if err := classify(ending{runErr: runErr, userStop: context.Cause(ctx) == errUserStop}); err != nil {
294	278		t.Fatalf("first frame cancel is a failure: %v", err)
295	279		}
296		-	if savedFile(path) {
297		-	t.Fatal("file exists")
	280	+	if _, err := os.Stat(path); !errors.Is(err, os.ErrNotExist) {
	281	+	t.Fatalf("expected no recording file: %v", err)
298	282		}
299	283		}
300	284


@@ -257,22 +257,6 @@ func TestFinishCapture(t *testing.T) {
257		}	257		}
258		}	258		}
259			259
260	-	func TestSavedFile(t *testing.T) {
261	-	dir := t.TempDir()
262	-	missing := filepath.Join(dir, "missing.mp4")
263	-	empty := filepath.Join(dir, "empty.mp4")
264	-	full := filepath.Join(dir, "full.mp4")
265	-	if err := os.WriteFile(empty, nil, 0o600); err != nil {
266	-	t.Fatal(err)
267	-	}
268	-	if err := os.WriteFile(full, []byte("x"), 0o600); err != nil {
269	-	t.Fatal(err)
270	-	}
271	-	if savedFile(missing) \|\| savedFile(empty) \|\| savedFile(dir) \|\| !savedFile(full) {
272	-	t.Fatal("saved file detection wrong")
273	-	}
274	-	}
275	-
276		// The production core with generated ports: cancelling during the first	260		// The production core with generated ports: cancelling during the first
277		// capture aborts nothing and writes nothing, and the lifecycle calls it success.	261		// capture aborts nothing and writes nothing, and the lifecycle calls it success.
278		func TestCancelDuringFirstFrameIsNotAFailure(t *testing.T) {	262		func TestCancelDuringFirstFrameIsNotAFailure(t *testing.T) {

@@ -293,8 +277,8 @@ func TestCancelDuringFirstFrameIsNotAFailure(t *testing.T) {
293		if err := classify(ending{runErr: runErr, userStop: context.Cause(ctx) == errUserStop}); err != nil {	277		if err := classify(ending{runErr: runErr, userStop: context.Cause(ctx) == errUserStop}); err != nil {
294		t.Fatalf("first frame cancel is a failure: %v", err)	278		t.Fatalf("first frame cancel is a failure: %v", err)
295		}	279		}
296	-	if savedFile(path) {	280	+	if _, err := os.Stat(path); !errors.Is(err, os.ErrNotExist) {
297	-	t.Fatal("file exists")	281	+	t.Fatalf("expected no recording file: %v", err)
298		}	282		}
299		}	283		}
300			284

M internal/app/recording_test.go +48 -4

@@ -1,12 +1,15 @@
1	1		package app
2	2
3	3		import (
	4	+	"bytes"
4	5		"context"
5	6		"encoding/json"
	7	+	"errors"
6	8		"os"
7	9		"os/exec"
8	10		"path/filepath"
9	11		"strconv"
	12	+	"strings"
10	13		"testing"
11	14		"time"
12	15

@@ -19,6 +22,19 @@ import (
19	22		// A generated source mock supplies pixels; the production core and FFmpeg
20	23		// writer run together, exercising cancellation at their owning boundary.
21	24		func TestRecordingFinalizesAfterCancellation(t *testing.T) {
	25	+	testRecordingFinalizes(t, "cancel")
	26	+	}
	27	+
	28	+	func TestRecordingFinalizesAfterLateCaptureFailure(t *testing.T) {
	29	+	testRecordingFinalizes(t, "capture")
	30	+	}
	31	+
	32	+	func TestRecordingFinalizesAfterLateGeometryFailure(t *testing.T) {
	33	+	testRecordingFinalizes(t, "geometry")
	34	+	}
	35	+
	36	+	func testRecordingFinalizes(t *testing.T, failure string) {
	37	+	t.Helper()
22	38		for _, binary := range []string{"ffmpeg", "ffprobe"} {
23	39		if _, err := exec.LookPath(binary); err != nil {
24	40		t.Skip(binary + " unavailable")

@@ -33,17 +49,45 @@ func TestRecordingFinalizesAfterCancellation(t *testing.T) {
33	49		target := ports.Target{OutputID: 1}
34	50		source := portsmocks.NewMockSource(t)
35	51		n := 0
	52	+	captureErr := errors.New("capture session lost")
36	53		source.EXPECT().Capture(ctx, target).RunAndReturn(func(context.Context, ports.Target) (ports.Frame, error) {
37	54		n++
38	55		if n == 5 {
39		-	cancel()
40		-	return ports.Frame{}, context.Canceled
	56	+	switch failure {
	57	+	case "capture":
	58	+	return ports.Frame{}, captureErr
	59	+	case "geometry":
	60	+	return ports.Frame{}, nil
	61	+	default:
	62	+	cancel()
	63	+	return ports.Frame{}, context.Canceled
	64	+	}
41	65		}
42	66		return frame, nil
43	67		})
44	68		path := filepath.Join(t.TempDir(), "recording.mp4")
45		-	if err := core.New(source, nil, ffmpeg.New()).Run(ctx, ports.Selection{Mode: ports.Record, Target: target, Path: path, Video: ports.VideoSettings{FPS: 30}}); err != nil {
46		-	t.Fatal(err)
	69	+	video := ffmpeg.New()
	70	+	runErr := core.New(source, nil, video).Run(ctx, ports.Selection{Mode: ports.Record, Target: target, Path: path, Video: ports.VideoSettings{FPS: 30}})
	71	+	switch failure {
	72	+	case "capture":
	73	+	if !errors.Is(runErr, captureErr) {
	74	+	t.Fatalf("capture failure lost: %v", runErr)
	75	+	}
	76	+	case "geometry":
	77	+	if runErr == nil \|\| !strings.Contains(runErr.Error(), "invalid frame dimensions") {
	78	+	t.Fatalf("geometry failure lost: %v", runErr)
	79	+	}
	80	+	default:
	81	+	if runErr != nil {
	82	+	t.Fatal(runErr)
	83	+	}
	84	+	}
	85	+	if !video.Saved {
	86	+	t.Fatal("finalized recording not reported as saved")
	87	+	}
	88	+	var out bytes.Buffer
	89	+	if err := finishCapture(&out, path, video.Saved, runErr); err != runErr \|\| out.String() != path+"\n" {
	90	+	t.Fatalf("retained path or recording error lost: %q %v", out.String(), err)
47	91		}
48	92		probeCtx, stop := context.WithTimeout(context.Background(), 5*time.Second)
49	93		defer stop()

@@ -1,12 +1,15 @@
1		package app	1		package app
2			2
3		import (	3		import (
			4	+	"bytes"
4		"context"	5		"context"
5		"encoding/json"	6		"encoding/json"
			7	+	"errors"
6		"os"	8		"os"
7		"os/exec"	9		"os/exec"
8		"path/filepath"	10		"path/filepath"
9		"strconv"	11		"strconv"
			12	+	"strings"
10		"testing"	13		"testing"
11		"time"	14		"time"
12			15

@@ -19,6 +22,19 @@ import (
19		// A generated source mock supplies pixels; the production core and FFmpeg	22		// A generated source mock supplies pixels; the production core and FFmpeg
20		// writer run together, exercising cancellation at their owning boundary.	23		// writer run together, exercising cancellation at their owning boundary.
21		func TestRecordingFinalizesAfterCancellation(t *testing.T) {	24		func TestRecordingFinalizesAfterCancellation(t *testing.T) {
			25	+	testRecordingFinalizes(t, "cancel")
			26	+	}
			27	+
			28	+	func TestRecordingFinalizesAfterLateCaptureFailure(t *testing.T) {
			29	+	testRecordingFinalizes(t, "capture")
			30	+	}
			31	+
			32	+	func TestRecordingFinalizesAfterLateGeometryFailure(t *testing.T) {
			33	+	testRecordingFinalizes(t, "geometry")
			34	+	}
			35	+
			36	+	func testRecordingFinalizes(t *testing.T, failure string) {
			37	+	t.Helper()
22		for _, binary := range []string{"ffmpeg", "ffprobe"} {	38		for _, binary := range []string{"ffmpeg", "ffprobe"} {
23		if _, err := exec.LookPath(binary); err != nil {	39		if _, err := exec.LookPath(binary); err != nil {
24		t.Skip(binary + " unavailable")	40		t.Skip(binary + " unavailable")

@@ -33,17 +49,45 @@ func TestRecordingFinalizesAfterCancellation(t *testing.T) {
33		target := ports.Target{OutputID: 1}	49		target := ports.Target{OutputID: 1}
34		source := portsmocks.NewMockSource(t)	50		source := portsmocks.NewMockSource(t)
35		n := 0	51		n := 0
			52	+	captureErr := errors.New("capture session lost")
36		source.EXPECT().Capture(ctx, target).RunAndReturn(func(context.Context, ports.Target) (ports.Frame, error) {	53		source.EXPECT().Capture(ctx, target).RunAndReturn(func(context.Context, ports.Target) (ports.Frame, error) {
37		n++	54		n++
38		if n == 5 {	55		if n == 5 {
39	-	cancel()	56	+	switch failure {
40	-	return ports.Frame{}, context.Canceled	57	+	case "capture":
			58	+	return ports.Frame{}, captureErr
			59	+	case "geometry":
			60	+	return ports.Frame{}, nil
			61	+	default:
			62	+	cancel()
			63	+	return ports.Frame{}, context.Canceled
			64	+	}
41		}	65		}
42		return frame, nil	66		return frame, nil
43		})	67		})
44		path := filepath.Join(t.TempDir(), "recording.mp4")	68		path := filepath.Join(t.TempDir(), "recording.mp4")
45	-	if err := core.New(source, nil, ffmpeg.New()).Run(ctx, ports.Selection{Mode: ports.Record, Target: target, Path: path, Video: ports.VideoSettings{FPS: 30}}); err != nil {	69	+	video := ffmpeg.New()
46	-	t.Fatal(err)	70	+	runErr := core.New(source, nil, video).Run(ctx, ports.Selection{Mode: ports.Record, Target: target, Path: path, Video: ports.VideoSettings{FPS: 30}})
			71	+	switch failure {
			72	+	case "capture":
			73	+	if !errors.Is(runErr, captureErr) {
			74	+	t.Fatalf("capture failure lost: %v", runErr)
			75	+	}
			76	+	case "geometry":
			77	+	if runErr == nil \|\| !strings.Contains(runErr.Error(), "invalid frame dimensions") {
			78	+	t.Fatalf("geometry failure lost: %v", runErr)
			79	+	}
			80	+	default:
			81	+	if runErr != nil {
			82	+	t.Fatal(runErr)
			83	+	}
			84	+	}
			85	+	if !video.Saved {
			86	+	t.Fatal("finalized recording not reported as saved")
			87	+	}
			88	+	var out bytes.Buffer
			89	+	if err := finishCapture(&out, path, video.Saved, runErr); err != runErr \|\| out.String() != path+"\n" {
			90	+	t.Fatalf("retained path or recording error lost: %q %v", out.String(), err)
47		}	91		}
48		probeCtx, stop := context.WithTimeout(context.Background(), 5*time.Second)	92		probeCtx, stop := context.WithTimeout(context.Background(), 5*time.Second)
49		defer stop()	93		defer stop()