fix(layout): close a half-read escape at the end of every frame so one broken frame cannot blank a module

commit: eff799ff376a93b416f53e809945b190ab934da0
author: brice · 17d ago · 2026-10-05 16:00 UTC
parent: 3fc69dd
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

4 files · +89 -3

M internal/layout/layout.go +5 -0


@@ -132,6 +132,11 @@ func (l *Layout) marker(dst []gpu.Cell, name string) []gpu.Cell {
132	132		func (s *Source) parse(fg, bg [3]uint8, pal [16][3]uint8) {
133	133		s.input = append(s.input[:0], "\x1b[0m\r\x1b[2K"...)
134	134		s.input = append(s.input, s.frame...)
	135	+	// A frame that ends inside an escape sequence (a script bug, or the frame
	136	+	// cap cutting one) would leave the parser waiting, and it would swallow the
	137	+	// next frames. ESC \ is the string terminator: it closes any such sequence,
	138	+	// and is harmless after a complete one.
	139	+	s.input = append(s.input, "\x1b\\"...)
135	140		s.screen.Write(s.input)
136	141		cols := s.screen.Columns()
137	142		// Trim trailing blank default cells so modules do not claim empty space.


@@ -132,6 +132,11 @@ func (l *Layout) marker(dst []gpu.Cell, name string) []gpu.Cell {
132	func (s *Source) parse(fg, bg [3]uint8, pal [16][3]uint8) {	132		func (s *Source) parse(fg, bg [3]uint8, pal [16][3]uint8) {
133	s.input = append(s.input[:0], "\x1b[0m\r\x1b[2K"...)	133		s.input = append(s.input[:0], "\x1b[0m\r\x1b[2K"...)
134	s.input = append(s.input, s.frame...)	134		s.input = append(s.input, s.frame...)
		135	+	// A frame that ends inside an escape sequence (a script bug, or the frame
		136	+	// cap cutting one) would leave the parser waiting, and it would swallow the
		137	+	// next frames. ESC \ is the string terminator: it closes any such sequence,
		138	+	// and is harmless after a complete one.
		139	+	s.input = append(s.input, "\x1b\\"...)
135	s.screen.Write(s.input)	140		s.screen.Write(s.input)
136	cols := s.screen.Columns()	141		cols := s.screen.Columns()
137	// Trim trailing blank default cells so modules do not claim empty space.	142		// Trim trailing blank default cells so modules do not claim empty space.

M internal/layout/layout_test.go +21 -0


@@ -163,3 +163,24 @@ func TestAnsiColorsComeFromThePalette(t *testing.T) {
163	163		t.Errorf("after SetColors, \\e[31m = %v, want %v", got, pal[1])
164	164		}
165	165		}
	166	+
	167	+	// A frame that stops inside an escape sequence must not blank the frames after it.
	168	+	func TestBrokenEscapeDoesNotSwallowNextFrames(t *testing.T) {
	169	+	for name, broken := range map[string]string{
	170	+	"osc": "\x1b]0;title",
	171	+	"csi": "\x1b[38;2;1",
	172	+	"dcs": "\x1bP1;2",
	173	+	"apc": "\x1b_Gf=24",
	174	+	"esc": "\x1b",
	175	+	"utf8": "ab\xe2\x82",
	176	+	} {
	177	+	l, m := newTestLayout(10)
	178	+	publish(m[0], broken)
	179	+	l.Update()
	180	+	publish(m[0], "hello")
	181	+	l.Update()
	182	+	if got := rowText(l); got[:5] != "hello" {
	183	+	t.Errorf("%s: row after the next frame = %q, want hello", name, got)
	184	+	}
	185	+	}
	186	+	}


@@ -163,3 +163,24 @@ func TestAnsiColorsComeFromThePalette(t *testing.T) {
163	t.Errorf("after SetColors, \\e[31m = %v, want %v", got, pal[1])	163		t.Errorf("after SetColors, \\e[31m = %v, want %v", got, pal[1])
164	}	164		}
165	}	165		}
		166	+
		167	+	// A frame that stops inside an escape sequence must not blank the frames after it.
		168	+	func TestBrokenEscapeDoesNotSwallowNextFrames(t *testing.T) {
		169	+	for name, broken := range map[string]string{
		170	+	"osc": "\x1b]0;title",
		171	+	"csi": "\x1b[38;2;1",
		172	+	"dcs": "\x1bP1;2",
		173	+	"apc": "\x1b_Gf=24",
		174	+	"esc": "\x1b",
		175	+	"utf8": "ab\xe2\x82",
		176	+	} {
		177	+	l, m := newTestLayout(10)
		178	+	publish(m[0], broken)
		179	+	l.Update()
		180	+	publish(m[0], "hello")
		181	+	l.Update()
		182	+	if got := rowText(l); got[:5] != "hello" {
		183	+	t.Errorf("%s: row after the next frame = %q, want hello", name, got)
		184	+	}
		185	+	}
		186	+	}

M internal/module/module.go +33 -3


@@ -52,6 +52,8 @@ type Module struct {
52	52
53	53		wake chan<- struct{}
54	54		log *slog.Logger
	55	+
	56	+	done sync.WaitGroup // Start adds one; Run releases it when its process is gone
55	57		}
56	58
57	59		// New creates a module. wake receives a non-blocking signal after each frame.

@@ -98,8 +100,28 @@ func (m *Module) setFailed() {
98	100		m.signal()
99	101		}
100	102
	103	+	// Start runs the module on its own goroutine until ctx ends. Wait returns once
	104	+	// the script and the processes it started are gone.
	105	+	func (m *Module) Start(ctx context.Context) {
	106	+	m.done.Add(1)
	107	+	go func() {
	108	+	defer m.done.Done()
	109	+	m.Run(ctx)
	110	+	}()
	111	+	}
	112	+
	113	+	// Wait blocks until a module started with Start has stopped, or timeout passes.
	114	+	func (m *Module) Wait(timeout time.Duration) {
	115	+	ch := make(chan struct{})
	116	+	go func() { m.done.Wait(); close(ch) }()
	117	+	select {
	118	+	case <-ch:
	119	+	case <-time.After(timeout):
	120	+	}
	121	+	}
	122	+
101	123		// Run starts the script and restarts it with backoff until ctx ends. It
102		-	// blocks, so call it on its own goroutine.
	124	+	// blocks, so call it on its own goroutine; use Start to also be able to Wait.
103	125		func (m *Module) Run(ctx context.Context) {
104	126		backoff := minBackoff
105	127		for ctx.Err() == nil {

@@ -137,7 +159,11 @@ func (m *Module) runOnce(ctx context.Context) error {
137	159		return err
138	160		}
139	161		m.readFrames(out)
140		-	return cmd.Wait()
	162	+	err = cmd.Wait()
	163	+	// The script may have started children that kept running (a "while" loop
	164	+	// in a subshell, a "sleep"). They share the process group: end them too.
	165	+	_ = syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL)
	166	+	return err
141	167		}
142	168
143	169		// readFrames splits r into frames and publishes each one.

@@ -178,7 +204,11 @@ type logWriter struct {
178	204		}
179	205
180	206		func (w *logWriter) Write(p []byte) (int, error) {
181		-	w.log.Info("module stderr", "module", w.name, "text", string(bytes.TrimRight(p, "\n")))
	207	+	text := bytes.TrimRight(p, "\n")
	208	+	if len(text) > 1024 {
	209	+	text = append(text[:1024:1024], "..."...)
	210	+	}
	211	+	w.log.Info("module stderr", "module", w.name, "text", string(text))
182	212		return len(p), nil
183	213		}
184	214


@@ -52,6 +52,8 @@ type Module struct {
52			52
53		wake chan<- struct{}	53		wake chan<- struct{}
54		log *slog.Logger	54		log *slog.Logger
			55	+
			56	+	done sync.WaitGroup // Start adds one; Run releases it when its process is gone
55		}	57		}
56			58
57		// New creates a module. wake receives a non-blocking signal after each frame.	59		// New creates a module. wake receives a non-blocking signal after each frame.

@@ -98,8 +100,28 @@ func (m *Module) setFailed() {
98		m.signal()	100		m.signal()
99		}	101		}
100			102
			103	+	// Start runs the module on its own goroutine until ctx ends. Wait returns once
			104	+	// the script and the processes it started are gone.
			105	+	func (m *Module) Start(ctx context.Context) {
			106	+	m.done.Add(1)
			107	+	go func() {
			108	+	defer m.done.Done()
			109	+	m.Run(ctx)
			110	+	}()
			111	+	}
			112	+
			113	+	// Wait blocks until a module started with Start has stopped, or timeout passes.
			114	+	func (m *Module) Wait(timeout time.Duration) {
			115	+	ch := make(chan struct{})
			116	+	go func() { m.done.Wait(); close(ch) }()
			117	+	select {
			118	+	case <-ch:
			119	+	case <-time.After(timeout):
			120	+	}
			121	+	}
			122	+
101		// Run starts the script and restarts it with backoff until ctx ends. It	123		// Run starts the script and restarts it with backoff until ctx ends. It
102	-	// blocks, so call it on its own goroutine.	124	+	// blocks, so call it on its own goroutine; use Start to also be able to Wait.
103		func (m *Module) Run(ctx context.Context) {	125		func (m *Module) Run(ctx context.Context) {
104		backoff := minBackoff	126		backoff := minBackoff
105		for ctx.Err() == nil {	127		for ctx.Err() == nil {

@@ -137,7 +159,11 @@ func (m *Module) runOnce(ctx context.Context) error {
137		return err	159		return err
138		}	160		}
139		m.readFrames(out)	161		m.readFrames(out)
140	-	return cmd.Wait()	162	+	err = cmd.Wait()
			163	+	// The script may have started children that kept running (a "while" loop
			164	+	// in a subshell, a "sleep"). They share the process group: end them too.
			165	+	_ = syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL)
			166	+	return err
141		}	167		}
142			168
143		// readFrames splits r into frames and publishes each one.	169		// readFrames splits r into frames and publishes each one.

@@ -178,7 +204,11 @@ type logWriter struct {
178		}	204		}
179			205
180		func (w *logWriter) Write(p []byte) (int, error) {	206		func (w *logWriter) Write(p []byte) (int, error) {
181	-	w.log.Info("module stderr", "module", w.name, "text", string(bytes.TrimRight(p, "\n")))	207	+	text := bytes.TrimRight(p, "\n")
			208	+	if len(text) > 1024 {
			209	+	text = append(text[:1024:1024], "..."...)
			210	+	}
			211	+	w.log.Info("module stderr", "module", w.name, "text", string(text))
182		return len(p), nil	212		return len(p), nil
183		}	213		}
184			214

M internal/module/module_test.go +30 -0


@@ -5,6 +5,8 @@ import (
5	5		"context"
6	6		"git.bnema.dev/bnema/neferbar/internal/racecheck"
7	7		"log/slog"
	8	+	"os"
	9	+	"path/filepath"
8	10		"strings"
9	11		"testing"
10	12		"time"

@@ -81,3 +83,31 @@ func TestPublishAllocs(t *testing.T) {
81	83		t.Errorf("publish+Take allocates %.1f objects; want 0", got)
82	84		}
83	85		}
	86	+
	87	+	// Stopping a module must also end the processes the script started, and Wait
	88	+	// must return once they are gone.
	89	+	func TestStopEndsTheScriptsChildren(t *testing.T) {
	90	+	marker := filepath.Join(t.TempDir(), "alive")
	91	+	// A background loop in a subshell: it outlives "sh" unless the whole group is killed.
	92	+	script := "(while :; do echo x > " + marker + "; sleep 0.05; done) & wait"
	93	+	m := New("t", Left, script, make(chan struct{}, 1), slog.New(slog.DiscardHandler))
	94	+	ctx, cancel := context.WithCancel(context.Background())
	95	+	m.Start(ctx)
	96	+	deadline := time.Now().Add(3 * time.Second)
	97	+	for {
	98	+	if _, err := os.Stat(marker); err == nil {
	99	+	break
	100	+	}
	101	+	if time.Now().After(deadline) {
	102	+	t.Fatal("the script never started")
	103	+	}
	104	+	time.Sleep(20 * time.Millisecond)
	105	+	}
	106	+	cancel()
	107	+	m.Wait(5 * time.Second)
	108	+	os.Remove(marker)
	109	+	time.Sleep(300 * time.Millisecond)
	110	+	if _, err := os.Stat(marker); err == nil {
	111	+	t.Fatal("a child of the script is still running after Wait returned")
	112	+	}
	113	+	}


@@ -5,6 +5,8 @@ import (
5	"context"	5		"context"
6	"git.bnema.dev/bnema/neferbar/internal/racecheck"	6		"git.bnema.dev/bnema/neferbar/internal/racecheck"
7	"log/slog"	7		"log/slog"
		8	+	"os"
		9	+	"path/filepath"
8	"strings"	10		"strings"
9	"testing"	11		"testing"
10	"time"	12		"time"

@@ -81,3 +83,31 @@ func TestPublishAllocs(t *testing.T) {
81	t.Errorf("publish+Take allocates %.1f objects; want 0", got)	83		t.Errorf("publish+Take allocates %.1f objects; want 0", got)
82	}	84		}
83	}	85		}
		86	+
		87	+	// Stopping a module must also end the processes the script started, and Wait
		88	+	// must return once they are gone.
		89	+	func TestStopEndsTheScriptsChildren(t *testing.T) {
		90	+	marker := filepath.Join(t.TempDir(), "alive")
		91	+	// A background loop in a subshell: it outlives "sh" unless the whole group is killed.
		92	+	script := "(while :; do echo x > " + marker + "; sleep 0.05; done) & wait"
		93	+	m := New("t", Left, script, make(chan struct{}, 1), slog.New(slog.DiscardHandler))
		94	+	ctx, cancel := context.WithCancel(context.Background())
		95	+	m.Start(ctx)
		96	+	deadline := time.Now().Add(3 * time.Second)
		97	+	for {
		98	+	if _, err := os.Stat(marker); err == nil {
		99	+	break
		100	+	}
		101	+	if time.Now().After(deadline) {
		102	+	t.Fatal("the script never started")
		103	+	}
		104	+	time.Sleep(20 * time.Millisecond)
		105	+	}
		106	+	cancel()
		107	+	m.Wait(5 * time.Second)
		108	+	os.Remove(marker)
		109	+	time.Sleep(300 * time.Millisecond)
		110	+	if _, err := os.Stat(marker); err == nil {
		111	+	t.Fatal("a child of the script is still running after Wait returned")
		112	+	}
		113	+	}