From 3a9e6ba68dd15997935f79f1ce623eccd40676bb Mon Sep 17 00:00:00 2001 From: Kevin Tang <73975146+vt128@users.noreply.github.com> Date: Sun, 20 Sep 2026 07:57:03 +0800 Subject: [PATCH 1/4] [fix] preserve session recordings and report write failures (STAR-106) --- README.md | 5 + cli/characterization_test.go | 4 + cli/dispatch.go | 4 +- cli/record.go | 100 ++++++++++++--- cli/wiring_test.go | 241 +++++++++++++++++++++++++++++++++++ e2e/e2e_test.go | 31 ++++- e2e/verify-release.py | 68 ++++++++++ go.mod | 2 +- 8 files changed, 432 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index c5ad6e6..b818e63 100644 --- a/README.md +++ b/README.md @@ -312,6 +312,11 @@ $ ./starcli --log-file run.log --log-format json job.star interaction and errors — to a transcript file (appended, with a timestamped session header), while still showing it live. Handy for replay and review. +REPL prompts and input echoes are included, so terminal transcripts can contain +ANSI control sequences. Recording errors produce a non-zero exit status; an +existing script error keeps its original exit code. If recording fails during +execution, output continues to drain and the failure is reported at shutdown. + ```bash $ ./starcli --record session.log job.star # works in REPL mode too $ cat session.log diff --git a/cli/characterization_test.go b/cli/characterization_test.go index 4a93a47..58f0559 100644 --- a/cli/characterization_test.go +++ b/cli/characterization_test.go @@ -47,10 +47,14 @@ func captureStd(t *testing.T, f func()) (stdout, stderr string) { if err != nil { t.Fatalf("pipe: %v", err) } + defer rOut.Close() + defer wOut.Close() rErr, wErr, err := os.Pipe() if err != nil { t.Fatalf("pipe: %v", err) } + defer rErr.Close() + defer wErr.Close() os.Stdout, os.Stderr = wOut, wErr outC, errC := make(chan string), make(chan string) go func() { var b bytes.Buffer; _, _ = io.Copy(&b, rOut); outC <- b.String() }() diff --git a/cli/dispatch.go b/cli/dispatch.go index 9eff702..882e88f 100644 --- a/cli/dispatch.go +++ b/cli/dispatch.go @@ -24,7 +24,7 @@ const ( // Process routes the parsed arguments to the desired run mode and returns the // process exit code. -func Process(args *Args) int { +func Process(args *Args) (code int) { // optionally record the whole session (stdout+stderr) to a transcript file if args.Record != "" { stop, err := startRecording(args.Record) @@ -32,7 +32,7 @@ func Process(args *Args) int { fmt.Fprintln(os.Stderr, err) return exitError } - defer stop() + defer func() { code = finishRecording(stop, code) }() } // for basic checks diff --git a/cli/record.go b/cli/record.go index 75580d4..ea1d486 100644 --- a/cli/record.go +++ b/cli/record.go @@ -1,21 +1,25 @@ package cli import ( + "errors" "fmt" "io" "os" "path/filepath" "sync" "time" + + "github.com/chzyer/readline" ) // startRecording tees the process's stdout and stderr to a transcript file, so a // whole session — print output, results, REPL interaction and errors — is saved // for later review. It returns a stop function that restores the streams and -// closes the file; call it before the process exits. stdin is left untouched, so +// closes the file and reports write/close errors; call it before the process +// exits. stdin and readline's terminal writers are preserved, so // an interactive REPL keeps its terminal. The file is appended to, with a // timestamped header delimiting each session. -func startRecording(path string) (stop func(), err error) { +func startRecording(path string) (stop func() error, err error) { if dir := filepath.Dir(path); dir != "" && dir != "." { if err := os.MkdirAll(dir, 0o755); err != nil { return nil, fmt.Errorf("record: %w", err) @@ -25,46 +29,104 @@ func startRecording(path string) (stop func(), err error) { if err != nil { return nil, fmt.Errorf("record: %w", err) } - fmt.Fprintf(f, "\n===== starcli session %s =====\n", time.Now().Format(time.RFC3339)) + return recordTo(f, os.Pipe) +} + +func recordTo(f io.WriteCloser, pipe func() (*os.File, *os.File, error)) (func() error, error) { + lf := &lockedWriter{w: f} + fmt.Fprintf(lf, "\n===== starcli session %s =====\n", time.Now().Format(time.RFC3339)) + if lf.err != nil { + _ = f.Close() + return nil, fmt.Errorf("record: %w", lf.err) + } - lf := &lockedWriter{w: f} // serialize the two streams into one file origOut, origErr := os.Stdout, os.Stderr - rOut, wOut, err := os.Pipe() + rOut, wOut, err := pipe() if err != nil { _ = f.Close() return nil, fmt.Errorf("record: %w", err) } - rErr, wErr, err := os.Pipe() + rErr, wErr, err := pipe() if err != nil { + _ = rOut.Close() _ = wOut.Close() _ = f.Close() return nil, fmt.Errorf("record: %w", err) } os.Stdout, os.Stderr = wOut, wErr + // readline captures its own writers at package initialization. Tee those + // separately, preserving its platform-specific terminal/ANSI handling. + origTermOut, origTermErr := readline.Stdout, readline.Stderr + readline.Stdout = borrowedWriter{io.MultiWriter(origTermOut, lf)} + readline.Stderr = borrowedWriter{io.MultiWriter(origTermErr, lf)} var wg sync.WaitGroup + var copyErrors [2]error wg.Add(2) - go func() { defer wg.Done(); _, _ = io.Copy(io.MultiWriter(origOut, lf), rOut) }() - go func() { defer wg.Done(); _, _ = io.Copy(io.MultiWriter(origErr, lf), rErr) }() + go func() { + defer wg.Done() + defer rOut.Close() + _, copyErrors[0] = io.Copy(io.MultiWriter(origOut, lf), rOut) + }() + go func() { + defer wg.Done() + defer rErr.Close() + _, copyErrors[1] = io.Copy(io.MultiWriter(origErr, lf), rErr) + }() - return func() { - os.Stdout, os.Stderr = origOut, origErr - _ = wOut.Close() - _ = wErr.Close() - wg.Wait() - _ = f.Close() + var once sync.Once + var stopErr error + return func() error { + once.Do(func() { + os.Stdout, os.Stderr = origOut, origErr + readline.Stdout, readline.Stderr = origTermOut, origTermErr + _ = wOut.Close() + _ = wErr.Close() + wg.Wait() + lf.mu.Lock() + err := errors.Join(lf.err, copyErrors[0], copyErrors[1], f.Close()) + lf.mu.Unlock() + if err != nil { + stopErr = fmt.Errorf("record: %w", err) + } + }) + return stopErr }, nil } -// lockedWriter serializes concurrent writes (the stdout and stderr copiers) to a -// single underlying writer so transcript lines are not interleaved mid-write. +func finishRecording(stop func() error, code int) int { + if err := stop(); err != nil { + fmt.Fprintln(os.Stderr, err) + if code == exitOK { + return exitError + } + } + return code +} + +// borrowedWriter must not close the caller's terminal stream. +type borrowedWriter struct{ io.Writer } + +func (borrowedWriter) Close() error { return nil } + +// lockedWriter serializes transcript writes and retains the first failure. +// It keeps accepting bytes after a recording failure so the pipe readers keep +// draining and a full disk cannot deadlock the script. stop reports the error. type lockedWriter struct { - mu sync.Mutex - w io.Writer + mu sync.Mutex + w io.Writer + err error } func (l *lockedWriter) Write(p []byte) (int, error) { l.mu.Lock() defer l.mu.Unlock() - return l.w.Write(p) + if l.err == nil { + var n int + n, l.err = l.w.Write(p) + if l.err == nil && n != len(p) { + l.err = io.ErrShortWrite + } + } + return len(p), nil } diff --git a/cli/wiring_test.go b/cli/wiring_test.go index 9a4482e..0ea1756 100644 --- a/cli/wiring_test.go +++ b/cli/wiring_test.go @@ -7,15 +7,23 @@ package cli // Sections: // - exit-code classification (success / eval / syntax / compile) // - execution budgets (--max-steps / --max-output) +// - session recording (terminal streams, failures, resource cleanup) // - check mode (--check: resolve without executing) import ( + "bytes" + "errors" + "fmt" + "io" "os" "path/filepath" + "runtime" + "runtime/debug" "strings" "testing" "github.com/1set/starbox" + "github.com/chzyer/readline" ) func TestProcess_ExitCodeClassification(t *testing.T) { @@ -237,6 +245,239 @@ func TestProcess_Record_CapturesErrors(t *testing.T) { } } +type recordingBuffer struct { + bytes.Buffer + closes int +} + +func (b *recordingBuffer) Close() error { b.closes++; return nil } + +func TestRecordReadlineStreams(t *testing.T) { + path := filepath.Join(t.TempDir(), "session.txt") + out, stderr := &recordingBuffer{}, &recordingBuffer{} + origOut, origErr := readline.Stdout, readline.Stderr + readline.Stdout, readline.Stderr = out, stderr + defer func() { readline.Stdout, readline.Stderr = origOut, origErr }() + stop, err := startRecording(path) + if err != nil { + t.Fatal(err) + } + fmt.Fprint(readline.Stdout, ">>> input echo\n") + fmt.Fprint(readline.Stderr, "terminal error\n") + _ = readline.Stdout.Close() + _ = readline.Stderr.Close() + if err := stop(); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{">>> input echo", "terminal error"} { + if !strings.Contains(string(data), want) { + t.Errorf("recording omitted %q: %q", want, data) + } + } + if out.String() != ">>> input echo\n" || stderr.String() != "terminal error\n" { + t.Fatalf("live terminal streams changed: %q / %q", out.String(), stderr.String()) + } + if readline.Stdout != out || readline.Stderr != stderr { + t.Fatal("terminal streams were not restored") + } + if out.closes != 0 || stderr.closes != 0 { + t.Fatal("recording closed borrowed terminal streams") + } +} + +func TestRecordClosesDescriptors(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("descriptor inventory is available on Unix") + } + oldGC := debug.SetGCPercent(-1) + defer debug.SetGCPercent(oldGC) + before, err := os.ReadDir("/dev/fd") + if err != nil { + t.Fatal(err) + } + path := filepath.Join(t.TempDir(), "session.txt") + for i := 0; i < 20; i++ { + stop, err := startRecording(path) + if err != nil { + t.Fatal(err) + } + if err := stop(); err != nil { + t.Fatal(err) + } + } + after, err := os.ReadDir("/dev/fd") + if err != nil { + t.Fatal(err) + } + // Allow the runtime poller to initialize; transcript pipes must not + // accumulate until a garbage collection happens to close them. + if len(after) > len(before)+2 { + t.Fatalf("recording leaked descriptors: before=%d after=%d", len(before), len(after)) + } +} + +func TestProcessRecordWriteFailure(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("/dev/full provides the disk-full fixture on Linux") + } + a := baseArgs() + a.Record = "/dev/full" + a.CodeContent = `print("must not run")` + var code int + out, stderr := captureStd(t, func() { code = Process(a) }) + if code != exitError || !strings.Contains(stderr, "record:") || strings.Contains(out, "must not run") { + t.Fatalf("record write failure: exit=%d stdout=%q stderr=%q", code, out, stderr) + } +} + +type recordingFile struct { + write func([]byte) (int, error) + close func() error +} + +func (f recordingFile) Write(p []byte) (int, error) { return f.write(p) } +func (f recordingFile) Close() error { return f.close() } + +func TestRecordSetupFailure(t *testing.T) { + for _, failAt := range []int{-1, 0, 1, 2} { + t.Run(fmt.Sprint(failAt), func(t *testing.T) { + var files []*os.File + calls, closes := 0, 0 + f := recordingFile{ + write: func(p []byte) (int, error) { + if failAt == -1 { + return len(p) - 1, nil + } + if failAt == 0 { + return 0, io.ErrClosedPipe + } + return len(p), nil + }, + close: func() error { closes++; return nil }, + } + out, stderr, termOut, termErr := os.Stdout, os.Stderr, readline.Stdout, readline.Stderr + stop, err := recordTo(f, func() (*os.File, *os.File, error) { + calls++ + if calls == failAt { + return nil, nil, io.ErrClosedPipe + } + r, w, err := os.Pipe() + if err == nil { + files = append(files, r, w) + } + return r, w, err + }) + wantErr := io.ErrClosedPipe + if failAt == -1 { + wantErr = io.ErrShortWrite + } + if !errors.Is(err, wantErr) || stop != nil || closes != 1 { + t.Fatalf("setup: err=%v has-stop=%v closes=%d", err, stop != nil, closes) + } + if os.Stdout != out || os.Stderr != stderr || readline.Stdout != termOut || readline.Stderr != termErr { + t.Fatal("failed setup changed streams") + } + for _, f := range files { + if _, err := f.Stat(); !errors.Is(err, os.ErrClosed) { + t.Errorf("pipe remained open: %v", err) + } + } + }) + } +} + +func TestRecordDrainAndClose(t *testing.T) { + for _, mode := range []string{"success", "write failure", "short write", "close failure", "stdout failure", "stderr failure"} { + t.Run(mode, func(t *testing.T) { + var files []*os.File + writes, closes := 0, 0 + f := recordingFile{ + write: func(p []byte) (int, error) { + writes++ + if writes > 1 && mode == "write failure" { + return 0, io.ErrClosedPipe + } + if writes > 1 && mode == "short write" { + return len(p) - 1, nil + } + return len(p), nil + }, + close: func() error { + closes++ + if mode == "close failure" { + return io.ErrClosedPipe + } + return nil + }, + } + var stopErr, againErr error + payload := strings.Repeat("record payload\n", 10000) + out, stderr := captureStd(t, func() { + if mode == "stdout failure" { + _ = os.Stdout.Close() + } + if mode == "stderr failure" { + _ = os.Stderr.Close() + } + stop, err := recordTo(f, func() (*os.File, *os.File, error) { + r, w, err := os.Pipe() + if err == nil { + files = append(files, r, w) + } + return r, w, err + }) + if err != nil { + t.Fatal(err) + } + _, _ = io.WriteString(os.Stdout, payload) + _, _ = io.WriteString(os.Stderr, payload) + stopErr, againErr = stop(), stop() + }) + if (stopErr == nil) != (mode == "success") || stopErr != againErr || closes != 1 { + t.Errorf("stop=%v again=%v closes=%d", stopErr, againErr, closes) + } + if mode == "short write" && !errors.Is(stopErr, io.ErrShortWrite) { + t.Errorf("short write was not reported: %v", stopErr) + } + if mode != "stdout failure" && out != payload || mode != "stderr failure" && stderr != payload { + t.Error("recording failure truncated live output") + } + for _, file := range files { + if _, err := file.Stat(); !errors.Is(err, os.ErrClosed) { + t.Errorf("pipe remained open: %v", err) + } + } + }) + } +} + +func TestFinishRecordingExitCodes(t *testing.T) { + for code := exitOK; code <= exitOutputLimit; code++ { + for _, fail := range []bool{false, true} { + var got int + _, stderr := captureStd(t, func() { + got = finishRecording(func() error { + if fail { + return errors.New("record: fixture failure") + } + return nil + }, code) + }) + want := code + if fail && code == exitOK { + want = exitError + } + if got != want || strings.Contains(stderr, "record: fixture failure") != fail { + t.Errorf("code=%d fail=%v: got=%d stderr=%q", code, fail, got, stderr) + } + } + } +} + // --- check mode (--check) ------------------------------------------------- func TestProcess_Check(t *testing.T) { diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go index fb9300f..f9bb767 100644 --- a/e2e/e2e_test.go +++ b/e2e/e2e_test.go @@ -4,7 +4,8 @@ // cost-price audit flagged as missing: it proves the wired modules actually // *work* through the CLI, not merely that they load. // -// Sections: local imports, golden scripts, domain modules, stdin, containers. +// Sections: local imports, golden scripts, domain modules, stdin, recordings, +// containers. package e2e import ( @@ -230,6 +231,34 @@ func TestStdinConsumption(t *testing.T) { } } +func TestSessionRecording(t *testing.T) { + path := filepath.Join(t.TempDir(), "session with spaces.log") + for _, tc := range []struct { + code string + exit int + }{ + {`print("recorded first run")`, 0}, + {`print("recorded second run"); fail("recorded error")`, 1}, + } { + _, stderr, exit := runCLI(t, "", "--record", path, "-c", tc.code) + if exit != tc.exit { + t.Fatalf("exit=%d want %d: %s", exit, tc.exit, stderr) + } + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{"recorded first run", "recorded second run", "recorded error"} { + if !strings.Contains(string(data), want) { + t.Errorf("transcript is missing %q", want) + } + } + if strings.Count(string(data), "===== starcli session") != 2 { + t.Errorf("expected two appended session headers: %s", data) + } +} + // TestContainer exercises the repository Dockerfile, including its entrypoint, // trust store, embedded time zones, non-root user, and PID 1 shutdown behavior. // Build it for linux/amd64 and set STARCLI_TEST_IMAGE to opt in. diff --git a/e2e/verify-release.py b/e2e/verify-release.py index 2d29b1b..89314d7 100644 --- a/e2e/verify-release.py +++ b/e2e/verify-release.py @@ -102,6 +102,73 @@ def smoke(binary, version): run(binary, "-c", 'fail("installation test")', success=False) +def verify_terminal_recording(binary, directory): + if os.name != "posix": + print("PTY recording test requires Unix; portable recording is covered by e2e", flush=True) + return + import pty + import select + import signal + import time + + for mode, flags in (("repl", []), ("inspect", ["-i", "-c", "value = 40"])): + transcript = directory / (mode + ".log") + pid, fd = pty.fork() + if pid == 0: + os.environ["TERM"] = "xterm-256color" + os.execv(str(binary), [str(binary), "--caps", "safe", "--record", str(transcript), *flags]) + output = bytearray() + + def receive(token, start=0): + deadline = time.monotonic() + 10 + while time.monotonic() < deadline: + if token in output[start:]: + return + if select.select([fd], [], [], 0.1)[0]: + chunk = os.read(fd, 65536) + if not chunk: + break + output.extend(chunk) + if len(output) > 1024 * 1024: + raise RuntimeError("unexpectedly large terminal output") + raise RuntimeError(f"missing terminal output {token!r}: {bytes(output)!r}") + + try: + receive(b">>> ") + for source, expected in ( + (b'print("recorded " + str(6*7))\n', b"recorded 42"), + (b'fail("terminal failure")\n', b"fail: terminal failure"), + (b'"recovered " + str(6*7)\n', b"recovered 42"), + (b'\x03', b"Interrupt"), + (b'print("after interrupt " + str(6*7))\n', b"after interrupt 42"), + ): + start = len(output) + os.write(fd, source) + receive(expected, start) + os.write(fd, b"\x04") + deadline = time.monotonic() + 10 + while time.monotonic() < deadline: + child, status = os.waitpid(pid, os.WNOHANG) + if child: + pid = None + if os.waitstatus_to_exitcode(status) != 0: + raise RuntimeError(f"terminal session failed: {status}") + break + time.sleep(0.05) + else: + raise RuntimeError("terminal session did not exit after EOF") + data = transcript.read_text() + for expected in (">>> ", "recorded 42", "fail: terminal failure", "recovered 42", "after interrupt 42"): + if expected not in data: + raise RuntimeError(f"transcript omitted {expected!r}") + print(f"passed real terminal recording: {mode}", flush=True) + finally: + os.close(fd) + if pid is not None: + os.kill(pid, signal.SIGKILL) + os.waitpid(pid, 0) + + def main(): directory, version, previous, previous_version = sys.argv[1:] candidate = verify_archives(Path(directory), version) @@ -123,6 +190,7 @@ def main(): ["go", "test", "-v", "./e2e", "-count=1"], check=True, env={**os.environ, "STARCLI_TEST_BINARY": str(binary)}, timeout=180, ) + verify_terminal_recording(binary, Path(temp)) if binary.read_bytes() != data: raise ValueError(f"installed bytes changed during {label}") print(f"passed {label}: v{expected_version}", flush=True) diff --git a/go.mod b/go.mod index b650c1c..697846b 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( bitbucket.org/ai69/colorlogo v0.1.3 github.com/1set/starbox v0.2.0 github.com/1set/starlet v0.2.3 + github.com/chzyer/readline v1.5.1 github.com/kyokomi/emoji/v2 v2.2.13 github.com/samber/lo v1.50.0 github.com/spf13/pflag v1.0.5 @@ -59,7 +60,6 @@ require ( github.com/charmbracelet/x/termios v0.1.1 // indirect github.com/charmbracelet/x/windows v0.2.2 // indirect github.com/chenzhuoyu/base64x v0.0.0-20221115062448-fe3a3abad311 // indirect - github.com/chzyer/readline v1.5.1 // indirect github.com/clipperhouse/displaywidth v0.11.0 // indirect github.com/clipperhouse/uax29/v2 v2.7.0 // indirect github.com/coder/websocket v1.8.12 // indirect From 71e19172773910bb13f093498a0c20e1b47cbbd5 Mon Sep 17 00:00:00 2001 From: Kevin Tang <73975146+vt128@users.noreply.github.com> Date: Sun, 20 Sep 2026 08:02:21 +0800 Subject: [PATCH 2/4] [fix] preserve session recordings and report write failures (STAR-106) --- cli/wiring_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/cli/wiring_test.go b/cli/wiring_test.go index 0ea1756..75458d8 100644 --- a/cli/wiring_test.go +++ b/cli/wiring_test.go @@ -334,6 +334,24 @@ func TestProcessRecordWriteFailure(t *testing.T) { } } +func TestProcessRecordPathErrors(t *testing.T) { + dir := t.TempDir() + file := filepath.Join(dir, "file") + if err := os.WriteFile(file, []byte("fixture"), 0o600); err != nil { + t.Fatal(err) + } + for _, path := range []string{dir, filepath.Join(file, "nested", "session.txt")} { + a := baseArgs() + a.Record = path + a.CodeContent = `print("must not run")` + var code int + out, stderr := captureStd(t, func() { code = Process(a) }) + if code != exitError || out != "" || !strings.Contains(stderr, "record:") { + t.Errorf("path=%q: exit=%d stdout=%q stderr=%q", path, code, out, stderr) + } + } +} + type recordingFile struct { write func([]byte) (int, error) close func() error From 7993939f1ba0bf79cb3a11059f227d1f7046556e Mon Sep 17 00:00:00 2001 From: Kevin Tang <73975146+vt128@users.noreply.github.com> Date: Sun, 20 Sep 2026 08:08:34 +0800 Subject: [PATCH 3/4] [test] verify closed recording resources across platforms --- cli/wiring_test.go | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/cli/wiring_test.go b/cli/wiring_test.go index 75458d8..f2a4dfb 100644 --- a/cli/wiring_test.go +++ b/cli/wiring_test.go @@ -295,7 +295,17 @@ func TestRecordClosesDescriptors(t *testing.T) { } oldGC := debug.SetGCPercent(-1) defer debug.SetGCPercent(oldGC) - before, err := os.ReadDir("/dev/fd") + listDescriptors := func() ([]string, error) { + dir, err := os.Open("/dev/fd") + if err != nil { + return nil, err + } + defer dir.Close() + // Enumerate names only: statting the virtual entries can race a + // descriptor closing, and older Darwin toolchains report EBADF. + return dir.Readdirnames(-1) + } + before, err := listDescriptors() if err != nil { t.Fatal(err) } @@ -309,7 +319,7 @@ func TestRecordClosesDescriptors(t *testing.T) { t.Fatal(err) } } - after, err := os.ReadDir("/dev/fd") + after, err := listDescriptors() if err != nil { t.Fatal(err) } @@ -400,8 +410,10 @@ func TestRecordSetupFailure(t *testing.T) { t.Fatal("failed setup changed streams") } for _, f := range files { - if _, err := f.Stat(); !errors.Is(err, os.ErrClosed) { - t.Errorf("pipe remained open: %v", err) + // Close must fail once the recording has already closed it. + // Stat's error for a closed Windows handle is not os.ErrClosed. + if err := f.Close(); err == nil { + t.Error("pipe remained open") } } }) @@ -465,8 +477,8 @@ func TestRecordDrainAndClose(t *testing.T) { t.Error("recording failure truncated live output") } for _, file := range files { - if _, err := file.Stat(); !errors.Is(err, os.ErrClosed) { - t.Errorf("pipe remained open: %v", err) + if err := file.Close(); err == nil { + t.Error("pipe remained open") } } }) From 6db0fe1d6b16e87fa9609964456e395432047e56 Mon Sep 17 00:00:00 2001 From: Kevin Tang <73975146+vt128@users.noreply.github.com> Date: Sun, 20 Sep 2026 08:20:13 +0800 Subject: [PATCH 4/4] [test] synchronize terminal control keys with readline input --- e2e/verify-release.py | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/e2e/verify-release.py b/e2e/verify-release.py index 89314d7..3fa556e 100644 --- a/e2e/verify-release.py +++ b/e2e/verify-release.py @@ -106,9 +106,11 @@ def verify_terminal_recording(binary, directory): if os.name != "posix": print("PTY recording test requires Unix; portable recording is covered by e2e", flush=True) return + import errno import pty import select import signal + import termios import time for mode, flags in (("repl", []), ("inspect", ["-i", "-c", "value = 40"])): @@ -119,18 +121,27 @@ def verify_terminal_recording(binary, directory): os.execv(str(binary), [str(binary), "--caps", "safe", "--record", str(transcript), *flags]) output = bytearray() + def read_output(timeout): + if not select.select([fd], [], [], timeout)[0]: + return + try: + output.extend(os.read(fd, 65536)) + except OSError as error: + # Linux reports EIO when the child closes its terminal. + if error.errno != errno.EIO: + raise + if len(output) > 1024 * 1024: + raise RuntimeError("unexpectedly large terminal output") + def receive(token, start=0): deadline = time.monotonic() + 10 while time.monotonic() < deadline: - if token in output[start:]: + # readline leaves raw mode while evaluating a command. Wait + # for the next read before sending control keys: canonical + # terminal handling can otherwise consume EOF prematurely. + if token in output[start:] and not termios.tcgetattr(fd)[3] & termios.ICANON: return - if select.select([fd], [], [], 0.1)[0]: - chunk = os.read(fd, 65536) - if not chunk: - break - output.extend(chunk) - if len(output) > 1024 * 1024: - raise RuntimeError("unexpectedly large terminal output") + read_output(0.1) raise RuntimeError(f"missing terminal output {token!r}: {bytes(output)!r}") try: @@ -154,9 +165,11 @@ def receive(token, start=0): if os.waitstatus_to_exitcode(status) != 0: raise RuntimeError(f"terminal session failed: {status}") break - time.sleep(0.05) + # Drain the terminal until exit, including final prompt/echo + # bytes, so the child cannot block on its output buffer. + read_output(0.05) else: - raise RuntimeError("terminal session did not exit after EOF") + raise RuntimeError(f"terminal session did not exit after EOF: {bytes(output[-4096:])!r}") data = transcript.read_text() for expected in (">>> ", "recorded 42", "fail: terminal failure", "recovered 42", "after interrupt 42"): if expected not in data: