detached ad771a6

test(capture): enforce allocation and frame validation guards

commit
ad771a64ae94061b8d74bbd48bc5a6258266773e
author
brice · · 2026-09-30 08:09 UTC
committer
brice · · 2026-09-30 14:48 UTC
parent
3cdbdaa
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
5 files · +52 -32
M AGENTS.md +1 -1
@@ -3,7 +3,7 @@
33 - Local-only repository: do not create remotes, publish branches, or access GitHub without explicit permission.
44 - Use Go 1.27; normal builds and tests use CGO_ENABLED=0. Enable CGO only for race tests.
55 - Follow lightweight hexagonal architecture: cmd/nefercap and internal/app wire internal/core, internal/ports, and internal/adapters. Core imports only stdlib and ports; ports contain no framework types.
6-- Use NeferGUI v0.10 for graphical interfaces and zerowrap with a component field for logging. Reuse NeferWL dependency versions where applicable; do not copy unrelated compositor dependencies.
6+- Use NeferGUI v0.1.0 for graphical interfaces and zerowrap with a component field for logging. Reuse NeferWL dependency versions where applicable; do not copy unrelated compositor dependencies.
77 - Work in .worktrees/ branches, not directly on main. Merge locally only after checks and review pass.
88 - Sign every commit and tag; commit format: type(scope): description.
99 - Minimize footprint. Bound buffers, queues and subprocess lifetimes. Reuse frame storage; test and benchmark steady-state allocations. Do not claim zero allocations without measurements.
@@ -3,7 +3,7 @@
3 - Local-only repository: do not create remotes, publish branches, or access GitHub without explicit permission.3 - Local-only repository: do not create remotes, publish branches, or access GitHub without explicit permission.
4 - Use Go 1.27; normal builds and tests use CGO_ENABLED=0. Enable CGO only for race tests.4 - Use Go 1.27; normal builds and tests use CGO_ENABLED=0. Enable CGO only for race tests.
5 - Follow lightweight hexagonal architecture: cmd/nefercap and internal/app wire internal/core, internal/ports, and internal/adapters. Core imports only stdlib and ports; ports contain no framework types.5 - Follow lightweight hexagonal architecture: cmd/nefercap and internal/app wire internal/core, internal/ports, and internal/adapters. Core imports only stdlib and ports; ports contain no framework types.
6-- Use NeferGUI v0.10 for graphical interfaces and zerowrap with a component field for logging. Reuse NeferWL dependency versions where applicable; do not copy unrelated compositor dependencies.6+- Use NeferGUI v0.1.0 for graphical interfaces and zerowrap with a component field for logging. Reuse NeferWL dependency versions where applicable; do not copy unrelated compositor dependencies.
7 - Work in .worktrees/ branches, not directly on main. Merge locally only after checks and review pass.7 - Work in .worktrees/ branches, not directly on main. Merge locally only after checks and review pass.
8 - Sign every commit and tag; commit format: type(scope): description.8 - Sign every commit and tag; commit format: type(scope): description.
9 - Minimize footprint. Bound buffers, queues and subprocess lifetimes. Reuse frame storage; test and benchmark steady-state allocations. Do not claim zero allocations without measurements.9 - Minimize footprint. Bound buffers, queues and subprocess lifetimes. Reuse frame storage; test and benchmark steady-state allocations. Do not claim zero allocations without measurements.
M Makefile +19 -17
@@ -1,4 +1,4 @@
1-.PHONY: bin build test vet race mocks mocks-check fakes-check arch perf-check check
1+.PHONY: bin build test vet race mod-check mocks mocks-check fakes-check arch perf-check check
22
33 GOBIN_DIR := $(or $(shell go env GOBIN),$(firstword $(subst :, ,$(shell go env GOPATH)))/bin)
44 MOCKERY ?= $(GOBIN_DIR)/mockery
@@ -16,6 +16,9 @@ vet:
1616 race:
1717 # cgo is enabled only for the race test binary.
1818 CGO_ENABLED=1 go test -race ./...
19+# go.mod and go.sum must be tidy (prints the diff and fails otherwise).
20+mod-check:
21+ go mod tidy -diff
1922
2023 # Regenerate from scratch so mocks of removed interfaces disappear too.
2124 mocks:
@@ -24,16 +27,9 @@ mocks:
2427 find internal -name '*_mock_test.go' -delete
2528 $(MOCKERY)
2629
27-# Checksum of all generated mocks (paths and contents); empty set is valid.
28-MOCKSUM = { find internal -type f \( -path 'internal/mocks/*' -o -name '*_mock_test.go' \) | LC_ALL=C sort | xargs -r sha256sum; } | sha256sum
2930 MOCKS := internal/mocks ':(glob)internal/**/*_mock_test.go'
30-# Regenerate and compare checksums (independent of committed state), then also
31-# require the mocks to be tracked and clean.
32-mocks-check:
33- @before=$$($(MOCKSUM)); \
34- $(MAKE) --no-print-directory mocks || exit $$?; \
35- after=$$($(MOCKSUM)); \
36- [ "$$before" = "$$after" ] || { echo 'generated mocks were stale: review and commit the regenerated files' >&2; exit 1; }
31+# Regenerate, then require the mocks to be tracked and unchanged.
32+mocks-check: mocks
3733 git diff --exit-code -- $(MOCKS)
3834 @test -z "$$(git ls-files --others --exclude-standard -- $(MOCKS))" || { echo 'untracked generated mocks: commit them' >&2; exit 1; }
3935
@@ -46,12 +42,18 @@ fakes-check:
4642 [ $$rc -eq 1 ] || { [ $$rc -eq 0 ] && echo 'handwritten test double: add an interface and a Mockery entry (see AGENTS.md)' >&2; exit 1; }
4743 arch:
4844 $(HEXCHECK) -hexcheck.config .hexcheck.yaml -hexcheck.root . ./...
49-# Steady-state allocation guards. Extend PERF_TESTS (regexp, exact names) and
50-# PERF_PKGS as encoder/wayland tests appear. The preflight fails if no listed
51-# test matches, so the guard cannot pass vacuously.
52-PERF_TESTS := ^TestFrameAllocations$$
45+# Steady-state allocation guards. PERF_TESTS lists exact test names and PERF_PKGS
46+# the packages holding them; extend both as encoder/wayland tests appear. Every
47+# named test must exist, so the guard cannot pass vacuously.
48+PERF_TESTS := TestFrameAllocations
5349 PERF_PKGS := ./internal/ports
50+perf_empty :=
51+perf_space := $(perf_empty) $(perf_empty)
52+PERF_RE := ^($(subst $(perf_space),|,$(strip $(PERF_TESTS))))$$
5453 perf-check:
55- @CGO_ENABLED=0 go test -list '$(PERF_TESTS)' $(PERF_PKGS) | grep -Eq '$(PERF_TESTS)' || { echo 'perf-check: no test matches $(PERF_TESTS) in $(PERF_PKGS)' >&2; exit 1; }
56- CGO_ENABLED=0 go test $(PERF_PKGS) -run '$(PERF_TESTS)' -count=1
57-check: vet test arch fakes-check perf-check
54+ @out=$$(CGO_ENABLED=0 go test -list '$(PERF_RE)' $(PERF_PKGS)) || { echo "$$out" >&2; exit 1; }; \
55+ for t in $(PERF_TESTS); do \
56+ printf '%s\n' "$$out" | grep -qx "$$t" || { echo "perf-check: test $$t not found in $(PERF_PKGS)" >&2; exit 1; }; \
57+ done
58+ CGO_ENABLED=0 go test $(PERF_PKGS) -run '$(PERF_RE)' -count=1
59+check: mod-check vet test arch fakes-check perf-check
@@ -1,4 +1,4 @@
1-.PHONY: bin build test vet race mocks mocks-check fakes-check arch perf-check check1+.PHONY: bin build test vet race mod-check mocks mocks-check fakes-check arch perf-check check
2 2
3 GOBIN_DIR := $(or $(shell go env GOBIN),$(firstword $(subst :, ,$(shell go env GOPATH)))/bin)3 GOBIN_DIR := $(or $(shell go env GOBIN),$(firstword $(subst :, ,$(shell go env GOPATH)))/bin)
4 MOCKERY ?= $(GOBIN_DIR)/mockery4 MOCKERY ?= $(GOBIN_DIR)/mockery
@@ -16,6 +16,9 @@ vet:
16 race:16 race:
17 # cgo is enabled only for the race test binary.17 # cgo is enabled only for the race test binary.
18 CGO_ENABLED=1 go test -race ./...18 CGO_ENABLED=1 go test -race ./...
19+# go.mod and go.sum must be tidy (prints the diff and fails otherwise).
20+mod-check:
21+ go mod tidy -diff
19 22
20 # Regenerate from scratch so mocks of removed interfaces disappear too.23 # Regenerate from scratch so mocks of removed interfaces disappear too.
21 mocks:24 mocks:
@@ -24,16 +27,9 @@ mocks:
24 find internal -name '*_mock_test.go' -delete27 find internal -name '*_mock_test.go' -delete
25 $(MOCKERY)28 $(MOCKERY)
26 29
27-# Checksum of all generated mocks (paths and contents); empty set is valid.
28-MOCKSUM = { find internal -type f \( -path 'internal/mocks/*' -o -name '*_mock_test.go' \) | LC_ALL=C sort | xargs -r sha256sum; } | sha256sum
29 MOCKS := internal/mocks ':(glob)internal/**/*_mock_test.go'30 MOCKS := internal/mocks ':(glob)internal/**/*_mock_test.go'
30-# Regenerate and compare checksums (independent of committed state), then also31+# Regenerate, then require the mocks to be tracked and unchanged.
31-# require the mocks to be tracked and clean.32+mocks-check: mocks
32-mocks-check:
33- @before=$$($(MOCKSUM)); \
34- $(MAKE) --no-print-directory mocks || exit $$?; \
35- after=$$($(MOCKSUM)); \
36- [ "$$before" = "$$after" ] || { echo 'generated mocks were stale: review and commit the regenerated files' >&2; exit 1; }
37 git diff --exit-code -- $(MOCKS)33 git diff --exit-code -- $(MOCKS)
38 @test -z "$$(git ls-files --others --exclude-standard -- $(MOCKS))" || { echo 'untracked generated mocks: commit them' >&2; exit 1; }34 @test -z "$$(git ls-files --others --exclude-standard -- $(MOCKS))" || { echo 'untracked generated mocks: commit them' >&2; exit 1; }
39 35
@@ -46,12 +42,18 @@ fakes-check:
46 [ $$rc -eq 1 ] || { [ $$rc -eq 0 ] && echo 'handwritten test double: add an interface and a Mockery entry (see AGENTS.md)' >&2; exit 1; }42 [ $$rc -eq 1 ] || { [ $$rc -eq 0 ] && echo 'handwritten test double: add an interface and a Mockery entry (see AGENTS.md)' >&2; exit 1; }
47 arch:43 arch:
48 $(HEXCHECK) -hexcheck.config .hexcheck.yaml -hexcheck.root . ./...44 $(HEXCHECK) -hexcheck.config .hexcheck.yaml -hexcheck.root . ./...
49-# Steady-state allocation guards. Extend PERF_TESTS (regexp, exact names) and45+# Steady-state allocation guards. PERF_TESTS lists exact test names and PERF_PKGS
50-# PERF_PKGS as encoder/wayland tests appear. The preflight fails if no listed46+# the packages holding them; extend both as encoder/wayland tests appear. Every
51-# test matches, so the guard cannot pass vacuously.47+# named test must exist, so the guard cannot pass vacuously.
52-PERF_TESTS := ^TestFrameAllocations$$48+PERF_TESTS := TestFrameAllocations
53 PERF_PKGS := ./internal/ports49 PERF_PKGS := ./internal/ports
50+perf_empty :=
51+perf_space := $(perf_empty) $(perf_empty)
52+PERF_RE := ^($(subst $(perf_space),|,$(strip $(PERF_TESTS))))$$
54 perf-check:53 perf-check:
55- @CGO_ENABLED=0 go test -list '$(PERF_TESTS)' $(PERF_PKGS) | grep -Eq '$(PERF_TESTS)' || { echo 'perf-check: no test matches $(PERF_TESTS) in $(PERF_PKGS)' >&2; exit 1; }54+ @out=$$(CGO_ENABLED=0 go test -list '$(PERF_RE)' $(PERF_PKGS)) || { echo "$$out" >&2; exit 1; }; \
56- CGO_ENABLED=0 go test $(PERF_PKGS) -run '$(PERF_TESTS)' -count=155+ for t in $(PERF_TESTS); do \
57-check: vet test arch fakes-check perf-check56+ printf '%s\n' "$$out" | grep -qx "$$t" || { echo "perf-check: test $$t not found in $(PERF_PKGS)" >&2; exit 1; }; \
57+ done
58+ CGO_ENABLED=0 go test $(PERF_PKGS) -run '$(PERF_RE)' -count=1
59+check: mod-check vet test arch fakes-check perf-check
M internal/ports/capture.go +1 -0
@@ -62,6 +62,7 @@ type VideoSettings struct {
6262 }
6363
6464 // VideoWriter has one owner. Start fixes input geometry and reserves a new path.
65+// Start does not encode its frame; pass the first frame explicitly to Write.
6566 // Write consumes borrowed pixels before returning; it must not queue frames.
6667 // Close drains and finalizes the encoder with a bounded shutdown deadline.
6768 // Abort interrupts pending writes and is safe concurrently with Write.
@@ -62,6 +62,7 @@ type VideoSettings struct {
62 }62 }
63 63
64 // VideoWriter has one owner. Start fixes input geometry and reserves a new path.64 // VideoWriter has one owner. Start fixes input geometry and reserves a new path.
65+// Start does not encode its frame; pass the first frame explicitly to Write.
65 // Write consumes borrowed pixels before returning; it must not queue frames.66 // Write consumes borrowed pixels before returning; it must not queue frames.
66 // Close drains and finalizes the encoder with a bounded shutdown deadline.67 // Close drains and finalizes the encoder with a bounded shutdown deadline.
67 // Abort interrupts pending writes and is safe concurrently with Write.68 // Abort interrupts pending writes and is safe concurrently with Write.
M internal/ports/errors.go +6 -2
@@ -1,6 +1,9 @@
11 package ports
22
3-import "errors"
3+import (
4+ "errors"
5+ "io/fs"
6+)
47
58 var (
69 ErrClosed = errors.New("capture resource is closed")
@@ -8,5 +11,6 @@ var (
811 ErrUnsupportedFormat = errors.New("unsupported capture pixel format")
912 ErrInvalidRegion = errors.New("invalid output-local capture region")
1013 ErrOutputNotFound = errors.New("capture output not found")
11- ErrPathExists = errors.New("capture path already exists")
14+ // ErrPathExists is the standard filesystem conflict sentinel.
15+ ErrPathExists = fs.ErrExist
1216 )
@@ -1,6 +1,9 @@
1 package ports1 package ports
2 2
3-import "errors"3+import (
4+ "errors"
5+ "io/fs"
6+)
4 7
5 var (8 var (
6 ErrClosed = errors.New("capture resource is closed")9 ErrClosed = errors.New("capture resource is closed")
@@ -8,5 +11,6 @@ var (
8 ErrUnsupportedFormat = errors.New("unsupported capture pixel format")11 ErrUnsupportedFormat = errors.New("unsupported capture pixel format")
9 ErrInvalidRegion = errors.New("invalid output-local capture region")12 ErrInvalidRegion = errors.New("invalid output-local capture region")
10 ErrOutputNotFound = errors.New("capture output not found")13 ErrOutputNotFound = errors.New("capture output not found")
11- ErrPathExists = errors.New("capture path already exists")14+ // ErrPathExists is the standard filesystem conflict sentinel.
15+ ErrPathExists = fs.ErrExist
12 )16 )
M internal/ports/frame_test.go +25 -12
@@ -1,6 +1,8 @@
11 package ports
22
33 import (
4+ "errors"
5+ "strings"
46 "testing"
57 )
68
@@ -10,24 +12,35 @@ func TestFrameValidation(t *testing.T) {
1012 t.Fatal(err)
1113 }
1214 cases := []struct {
13- name string
14- change func(*Frame)
15+ name string
16+ frame Frame
17+ wantMessage string
18+ wantIs error
1519 }{
16- {"zero width", func(f *Frame) { f.Width = 0 }},
17- {"negative height", func(f *Frame) { f.Height = -1 }},
18- {"oversized width", func(f *Frame) { f.Width = MaxDimension + 1 }},
19- {"short stride", func(f *Frame) { f.Stride = 7 }},
20- {"oversized storage", func(f *Frame) { f.Stride = MaxFrameBytes }},
21- {"unsupported format", func(f *Frame) { f.Format = 42 }},
22- {"short storage", func(f *Frame) { f.Pixels = f.Pixels[:23] }},
20+ {"zero width", Frame{Width: 0, Height: 1, Stride: 4, Pixels: make([]byte, 4)}, "invalid frame dimensions", nil},
21+ {"negative height", Frame{Width: 1, Height: -1, Stride: 4, Pixels: make([]byte, 4)}, "invalid frame dimensions", nil},
22+ {"oversized width", Frame{Width: MaxDimension + 1, Height: 1, Stride: (MaxDimension + 1) * 4, Pixels: make([]byte, (MaxDimension+1)*4)}, "invalid frame dimensions", nil},
23+ {"oversized height", Frame{Width: 1, Height: MaxDimension + 1, Stride: 4, Pixels: make([]byte, (MaxDimension+1)*4)}, "invalid frame dimensions", nil},
24+ {"short aligned stride", Frame{Width: 2, Height: 1, Stride: 4, Pixels: make([]byte, 4)}, "invalid frame stride", nil},
25+ {"unaligned stride", Frame{Width: 1, Height: 1, Stride: 6, Pixels: make([]byte, 6)}, "invalid frame stride", nil},
26+ // The error family proves storage bounds run before the short-slice check.
27+ {"excessive stride", Frame{Width: 1, Height: 2, Stride: MaxFrameBytes/2 + 4}, "invalid frame stride", nil},
28+ {"overflow stride", Frame{Width: 1, Height: 2, Stride: int(^uint(0)>>1) &^ 3}, "invalid frame stride", nil},
29+ {"unsupported format", Frame{Width: 1, Height: 1, Stride: 4, Pixels: make([]byte, 4), Format: 42}, "", ErrUnsupportedFormat},
30+ {"short storage", Frame{Width: 2, Height: 2, Stride: 12, Pixels: make([]byte, 23)}, "short frame storage", nil},
2331 }
2432 for _, tc := range cases {
2533 t.Run(tc.name, func(t *testing.T) {
26- f := valid
27- tc.change(&f)
28- if f.Validate() == nil {
34+ err := tc.frame.Validate()
35+ if err == nil {
2936 t.Fatal("accepted invalid frame")
3037 }
38+ if tc.wantIs != nil && !errors.Is(err, tc.wantIs) {
39+ t.Fatalf("got %v, want %v", err, tc.wantIs)
40+ }
41+ if tc.wantMessage != "" && !strings.Contains(err.Error(), tc.wantMessage) {
42+ t.Fatalf("got %v, want %s", err, tc.wantMessage)
43+ }
3144 })
3245 }
3346 }
@@ -1,6 +1,8 @@
1 package ports1 package ports
2 2
3 import (3 import (
4+ "errors"
5+ "strings"
4 "testing"6 "testing"
5 )7 )
6 8
@@ -10,24 +12,35 @@ func TestFrameValidation(t *testing.T) {
10 t.Fatal(err)12 t.Fatal(err)
11 }13 }
12 cases := []struct {14 cases := []struct {
13- name string15+ name string
14- change func(*Frame)16+ frame Frame
17+ wantMessage string
18+ wantIs error
15 }{19 }{
16- {"zero width", func(f *Frame) { f.Width = 0 }},20+ {"zero width", Frame{Width: 0, Height: 1, Stride: 4, Pixels: make([]byte, 4)}, "invalid frame dimensions", nil},
17- {"negative height", func(f *Frame) { f.Height = -1 }},21+ {"negative height", Frame{Width: 1, Height: -1, Stride: 4, Pixels: make([]byte, 4)}, "invalid frame dimensions", nil},
18- {"oversized width", func(f *Frame) { f.Width = MaxDimension + 1 }},22+ {"oversized width", Frame{Width: MaxDimension + 1, Height: 1, Stride: (MaxDimension + 1) * 4, Pixels: make([]byte, (MaxDimension+1)*4)}, "invalid frame dimensions", nil},
19- {"short stride", func(f *Frame) { f.Stride = 7 }},23+ {"oversized height", Frame{Width: 1, Height: MaxDimension + 1, Stride: 4, Pixels: make([]byte, (MaxDimension+1)*4)}, "invalid frame dimensions", nil},
20- {"oversized storage", func(f *Frame) { f.Stride = MaxFrameBytes }},24+ {"short aligned stride", Frame{Width: 2, Height: 1, Stride: 4, Pixels: make([]byte, 4)}, "invalid frame stride", nil},
21- {"unsupported format", func(f *Frame) { f.Format = 42 }},25+ {"unaligned stride", Frame{Width: 1, Height: 1, Stride: 6, Pixels: make([]byte, 6)}, "invalid frame stride", nil},
22- {"short storage", func(f *Frame) { f.Pixels = f.Pixels[:23] }},26+ // The error family proves storage bounds run before the short-slice check.
27+ {"excessive stride", Frame{Width: 1, Height: 2, Stride: MaxFrameBytes/2 + 4}, "invalid frame stride", nil},
28+ {"overflow stride", Frame{Width: 1, Height: 2, Stride: int(^uint(0)>>1) &^ 3}, "invalid frame stride", nil},
29+ {"unsupported format", Frame{Width: 1, Height: 1, Stride: 4, Pixels: make([]byte, 4), Format: 42}, "", ErrUnsupportedFormat},
30+ {"short storage", Frame{Width: 2, Height: 2, Stride: 12, Pixels: make([]byte, 23)}, "short frame storage", nil},
23 }31 }
24 for _, tc := range cases {32 for _, tc := range cases {
25 t.Run(tc.name, func(t *testing.T) {33 t.Run(tc.name, func(t *testing.T) {
26- f := valid34+ err := tc.frame.Validate()
27- tc.change(&f)35+ if err == nil {
28- if f.Validate() == nil {
29 t.Fatal("accepted invalid frame")36 t.Fatal("accepted invalid frame")
30 }37 }
38+ if tc.wantIs != nil && !errors.Is(err, tc.wantIs) {
39+ t.Fatalf("got %v, want %v", err, tc.wantIs)
40+ }
41+ if tc.wantMessage != "" && !strings.Contains(err.Error(), tc.wantMessage) {
42+ t.Fatalf("got %v, want %s", err, tc.wantMessage)
43+ }
31 })44 })
32 }45 }
33 }46 }

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 2

Tags 9

↑↓ moveenter openesc close