From 8a135b7ad6767d18f67d057f909f4535d6daff15 Mon Sep 17 00:00:00 2001 From: omartelo Date: Fri, 23 Oct 2026 16:54:42 -0300 Subject: [PATCH 1/4] fix(terminal): seed a reopened terminal from a replay frame in line with its output terminal.Replay queues the session's tail on /ws as a replay frame, behind the output already emitted and ahead of the rest, and drops what the coalescer held, which the tail already carries. The window registers the terminal before asking, takes no output until the replay frame lands, and sends none of the terminal's replies to queries inside the tail. Before, the window wrote the tail returned by the RPC into a terminal wired to the PTY: the answers to queries in it (nvim's device attributes, cursor reports) were typed into the session, and output emitted between the snapshot and the terminal's registration was lost. The RPC now returns whether a replay frame is coming. Tests that read the tail through Replay read the buffer directly: the contract changed. --- CHANGELOG.md | 2 + api/terminal.go | 2 +- api/testdata/contract.golden | 2 +- internal/terminal/coalescer.go | 17 ++++ internal/terminal/io.go | 26 ++++-- internal/terminal/outbox.go | 30 +++++-- internal/terminal/outbox_test.go | 6 +- internal/terminal/replay_test.go | 51 ++++++----- internal/terminal/respawn_test.go | 18 ++-- internal/terminal/sandbox_spawn_linux_test.go | 14 +--- internal/terminal/start.go | 6 ++ internal/terminal/terminal.go | 4 + internal/terminal/transport.go | 29 +++++++ native/backend/api.go | 20 ++--- native/backend/client.go | 16 ++-- native/backend/live_test.go | 2 +- native/cmd/lich-native/main.go | 2 +- native/cmd/lich-native/terminals.go | 84 +++++++++++++------ .../cmd/lich-native/terminals_colors_test.go | 2 +- .../cmd/lich-native/terminals_replay_test.go | 64 ++++++++++++++ 20 files changed, 290 insertions(+), 107 deletions(-) create mode 100644 native/cmd/lich-native/terminals_replay_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 432af6c7c..250134f01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Kiro CLI sessions follow `KIRO_HOME`. With it set, lich read Kiro's conversations, cost and context from `~/.kiro` while Kiro kept them under `KIRO_HOME`, so they showed nothing and History listed none; the agent that carries lich's hooks was installed where Kiro never looked, and a sandboxed session was not given the directory it writes into. - Opening or closing the dock no longer leaves the session's terminal cut off at its edge for a beat once the dock stops. The terminal takes its new width as the dock starts moving, so the agent redraws while the dock slides in, where the terminal waited until a tenth of a second after the dock stopped and then jumped. A split layout's terminals fit the moment the dock stops. - Expanding the sidebar no longer leaves the session's terminal at its old width for a beat once the sidebar is open. The terminal takes its new width as the sidebar starts moving, as with the dock, where it waited until a tenth of a second after the sidebar stopped and then jumped; collapsing it refits at once too. A split layout's terminals fit the moment the sidebar stops. +- Reopening the window no longer types stray text into a session. Rebuilding a terminal from its recent output answered the terminal queries in it again, such as the ones nvim sends at start, and the answers landed on the session's input: in a shell as a `command not found`, in an agent as text in its prompt. +- A session's terminal opened while the session is printing no longer misses what it printed in that moment. The backend now hands the window the session's recent output in line with its live output, so nothing between the two is lost or shown twice. ## [0.78.0] - 2026-10-22 diff --git a/api/terminal.go b/api/terminal.go index e99f8f2f5..32084581a 100644 --- a/api/terminal.go +++ b/api/terminal.go @@ -6,7 +6,7 @@ var ( TerminalWrite = define[WriteOptions, None]("terminal.Write") TerminalResize = define[ResizeOptions, None]("terminal.Resize") TerminalSetVisible = define[SetVisibleOptions, None]("terminal.SetVisible") - TerminalReplay = define[SessionIDOptions, string]("terminal.Replay") + TerminalReplay = define[SessionIDOptions, bool]("terminal.Replay") TerminalLive = define[SessionIDOptions, bool]("terminal.Live") TerminalLiveCount = define[None, int]("terminal.LiveCount") TerminalReady = define[SessionIDOptions, bool]("terminal.Ready") diff --git a/api/testdata/contract.golden b/api/testdata/contract.golden index e9f87f8f8..ffb75d186 100644 --- a/api/testdata/contract.golden +++ b/api/testdata/contract.golden @@ -177,7 +177,7 @@ terminal.LastTurnSaid(SessionIDOptions) LastSaid terminal.Live(SessionIDOptions) bool terminal.LiveCount(None) int terminal.Ready(SessionIDOptions) bool -terminal.Replay(SessionIDOptions) string +terminal.Replay(SessionIDOptions) bool terminal.Resize(ResizeOptions) None terminal.ResumeAvailable(ResumeAvailableOptions) bool terminal.SearchTranscripts(SearchTranscriptsOptions) []TranscriptMatch diff --git a/internal/terminal/coalescer.go b/internal/terminal/coalescer.go index 193d79c05..2051c9e01 100644 --- a/internal/terminal/coalescer.go +++ b/internal/terminal/coalescer.go @@ -106,6 +106,23 @@ func (c *coalescer) Close() { c.flushLocked() } +// cut drops the pending output and runs then with the coalescer held, so no +// output is emitted between the two: the replay a cut queues already holds +// what was pending. A closed coalescer runs nothing. +func (c *coalescer) cut(then func()) { + c.mu.Lock() + defer c.mu.Unlock() + if c.closed { + return + } + if c.timer != nil { + c.timer.Stop() + c.timer = nil + } + c.pending = nil + then() +} + // flushLocked emits pending output and disarms the timer. Caller holds mu. func (c *coalescer) flushLocked() { if c.timer != nil { diff --git a/internal/terminal/io.go b/internal/terminal/io.go index 7aaa3c7b0..5fc0d385b 100644 --- a/internal/terminal/io.go +++ b/internal/terminal/io.go @@ -2,7 +2,6 @@ package terminal import ( "context" - "encoding/base64" "math" "time" @@ -25,8 +24,10 @@ func (s *Service) stream(id string, sess *session) { n, err := p.Read(buf) if n > 0 { s.noteOutput(id, sess, buf[:n]) + sess.feed.Lock() sess.replay.append(buf[:n]) sess.out.Write(buf[:n]) + sess.feed.Unlock() } if err != nil { break @@ -138,19 +139,26 @@ func (s *Service) SetVisible(_ context.Context, opts api.SetVisibleOptions) (api return api.None{}, nil } -// Replay returns the capped tail of a session's PTY output, base64-encoded, so a -// window attaching to a running session can reseed a fresh terminal's -// scrollback. Empty for an unknown session — a brand-new one has no -// history yet. Base64 for the same reason the data event is: raw PTY bytes may -// split a multi-byte UTF-8 sequence across the JSON envelope. -func (s *Service) Replay(_ context.Context, opts api.SessionIDOptions) (string, error) { +// Replay queues the capped tail of a session's PTY output on /ws as a replay +// frame, so a window attaching to a running session can reseed a fresh +// terminal's scrollback: the output frames before it are in the tail, and the +// ones after it are not. It reports whether a replay frame is coming, false +// for an unknown session, which a brand-new one is: it has no history yet. +func (s *Service) Replay(_ context.Context, opts api.SessionIDOptions) (bool, error) { s.mu.Lock() sess, ok := s.sessions[opts.ID] s.mu.Unlock() if !ok { - return "", nil + return false, nil } - return base64.StdEncoding.EncodeToString(sess.replay.snapshot()), nil + queued := false + sess.feed.Lock() + defer sess.feed.Unlock() + sess.out.cut(func() { + sess.outbox.pushReplay(sess.replay.snapshot()) + queued = true + }) + return queued, nil } // Resize updates a session's PTY window size. The window only calls this for diff --git a/internal/terminal/outbox.go b/internal/terminal/outbox.go index 000374a2f..3ffc57d60 100644 --- a/internal/terminal/outbox.go +++ b/internal/terminal/outbox.go @@ -18,18 +18,28 @@ const outboxDepth = 32 // once the queue is full, that session's PTY. The backpressure that remains is // honest — it reaches the session that is producing, never its neighbours. type outbox struct { - frames chan []byte + frames chan outFrame done chan struct{} } -// newOutbox starts the delivery goroutine. deliver is called with one frame at -// a time, in order, and owns the slice it is handed. -func newOutbox(deliver func(data []byte), depth int) *outbox { - o := &outbox{frames: make(chan []byte, depth), done: make(chan struct{})} +type outFrame struct { + data []byte + replay bool +} + +// newOutbox starts the delivery goroutine. deliver is called with each output +// frame and replay with each replay frame, one at a time and in the order they +// were queued; both own the slice they are handed. +func newOutbox(deliver, replay func(data []byte), depth int) *outbox { + o := &outbox{frames: make(chan outFrame, depth), done: make(chan struct{})} go func() { defer close(o.done) for frame := range o.frames { - deliver(frame) + if frame.replay { + replay(frame.data) + continue + } + deliver(frame.data) } }() return o @@ -38,7 +48,13 @@ func newOutbox(deliver func(data []byte), depth int) *outbox { // push queues one frame for delivery, blocking only while this session's queue // is full. The frame is copied: the coalescer reuses the buffer it passes. func (o *outbox) push(data []byte) { - o.frames <- append([]byte(nil), data...) + o.frames <- outFrame{data: append([]byte(nil), data...)} +} + +// pushReplay queues the session's replayed tail behind every frame already +// queued, so whoever reads it knows what came before the tail and what after. +func (o *outbox) pushReplay(tail []byte) { + o.frames <- outFrame{data: tail, replay: true} } // close stops the queue and waits for the frames still in it to be delivered, diff --git a/internal/terminal/outbox_test.go b/internal/terminal/outbox_test.go index 136b361c4..4e2150adf 100644 --- a/internal/terminal/outbox_test.go +++ b/internal/terminal/outbox_test.go @@ -21,7 +21,7 @@ func TestOutboxQueuesWhileDeliveryIsStuck(t *testing.T) { default: } <-release - }, 2) + }, nil, 2) // The first frame is taken by the (stuck) delivery goroutine; two more fit // the queue. @@ -68,7 +68,7 @@ func TestOutboxDeliversInOrderAndDrainsOnClose(t *testing.T) { mu.Lock() got = append(got, string(data)) mu.Unlock() - }, 4) + }, nil, 4) for _, frame := range []string{"a", "b", "c", "d", "e"} { box.push([]byte(frame)) @@ -87,7 +87,7 @@ func TestOutboxDeliversInOrderAndDrainsOnClose(t *testing.T) { func TestOutboxCopiesTheFrame(t *testing.T) { t.Parallel() delivered := make(chan []byte, 1) - box := newOutbox(func(data []byte) { delivered <- data }, 1) + box := newOutbox(func(data []byte) { delivered <- data }, nil, 1) buf := []byte("live") box.push(buf) diff --git a/internal/terminal/replay_test.go b/internal/terminal/replay_test.go index 4ff9b3b9a..0ff14f8f8 100644 --- a/internal/terminal/replay_test.go +++ b/internal/terminal/replay_test.go @@ -2,10 +2,11 @@ package terminal import ( "bytes" - "encoding/base64" "github.com/lichdotdev/lich/api" + "slices" "strings" "testing" + "time" ) func TestReplayBufferKeepsOutputInOrder(t *testing.T) { @@ -65,34 +66,46 @@ func TestReplayBufferKeepsOneChunkLargerThanCap(t *testing.T) { } // A bare Service with a hand-built session exercises Replay without a PTY, so -// this stays OS-agnostic (no !windows build tag): Replay only reads the tail. -func TestServiceReplayEncodesTheTail(t *testing.T) { +// this stays OS-agnostic (no !windows build tag). The replay frame lands +// between the output already emitted and the output after it, and what the +// coalescer was holding goes out once, inside the tail, never again after it. +func TestServiceReplayQueuesTheTailBetweenOutputFrames(t *testing.T) { t.Parallel() - b := newReplayBuffer(replayCapBytes) - b.append([]byte("scrollback")) - svc := &Service{sessions: map[string]*session{"s1": {replay: b}}} - - encoded, err := svc.Replay(t.Context(), api.SessionIDOptions{ID: "s1"}) - if err != nil { - t.Fatalf("Replay = %v, want nil", err) + var got []string + box := newOutbox( + func(data []byte) { got = append(got, "out:"+string(data)) }, + func(tail []byte) { got = append(got, "replay:"+string(tail)) }, + outboxDepth) + sess := &session{replay: newReplayBuffer(replayCapBytes), outbox: box, out: newCoalescer(box.push, time.Hour, time.Hour)} + read := func(data string) { + sess.replay.append([]byte(data)) + sess.out.Write([]byte(data)) } - decoded, err := base64.StdEncoding.DecodeString(encoded) - if err != nil { - t.Fatalf("decode: %v", err) + svc := &Service{sessions: map[string]*session{"s1": sess}} + + read("a") + read("b") + queued, err := svc.Replay(t.Context(), api.SessionIDOptions{ID: "s1"}) + if err != nil || !queued { + t.Fatalf("Replay = %v, %v; want a replay queued", queued, err) } - if string(decoded) != "scrollback" { - t.Fatalf("Replay tail = %q, want %q", decoded, "scrollback") + read("c") + sess.out.Close() + box.close() + + if want := []string{"out:a", "replay:ab", "out:c"}; !slices.Equal(got, want) { + t.Fatalf("delivered %q, want %q", got, want) } } -func TestServiceReplayUnknownSessionIsEmpty(t *testing.T) { +func TestServiceReplayUnknownSessionQueuesNothing(t *testing.T) { t.Parallel() svc := &Service{sessions: map[string]*session{}} - got, err := svc.Replay(t.Context(), api.SessionIDOptions{ID: "nope"}) + queued, err := svc.Replay(t.Context(), api.SessionIDOptions{ID: "nope"}) if err != nil { t.Fatalf("Replay = %v, want nil", err) } - if got != "" { - t.Fatalf("Replay unknown = %q, want empty", got) + if queued { + t.Fatal("Replay of an unknown session reported a replay coming") } } diff --git a/internal/terminal/respawn_test.go b/internal/terminal/respawn_test.go index 70304f068..55a55384e 100644 --- a/internal/terminal/respawn_test.go +++ b/internal/terminal/respawn_test.go @@ -5,7 +5,6 @@ package terminal import ( - "encoding/base64" "fmt" "os" "path/filepath" @@ -174,18 +173,17 @@ func exitBin(t *testing.T, code int) string { return path } -// replayOf returns a session's replay buffer decoded back to raw output. +// replayOf returns a session's replay buffer as raw output, "" for a session +// not running. func replayOf(t *testing.T, svc *Service, id string) string { t.Helper() - encoded, err := svc.Replay(t.Context(), api.SessionIDOptions{ID: id}) - if err != nil { - t.Fatalf("replay: %v", err) - } - raw, err := base64.StdEncoding.DecodeString(encoded) - if err != nil { - t.Fatalf("decode replay: %v", err) + svc.mu.Lock() + sess, ok := svc.sessions[id] + svc.mu.Unlock() + if !ok { + return "" } - return string(raw) + return string(sess.replay.snapshot()) } // A PTY that dies with its turn open, the agent crashing or the shell exiting diff --git a/internal/terminal/sandbox_spawn_linux_test.go b/internal/terminal/sandbox_spawn_linux_test.go index 899a90b23..b3e6e6b58 100644 --- a/internal/terminal/sandbox_spawn_linux_test.go +++ b/internal/terminal/sandbox_spawn_linux_test.go @@ -3,7 +3,6 @@ package terminal import ( - "encoding/base64" "os" "path/filepath" "slices" @@ -52,19 +51,12 @@ func TestAConfinedSessionRunsInItsPTY(t *testing.T) { var answer string waitFor(t, func() bool { - encoded, err := svc.Replay(t.Context(), api.SessionIDOptions{ID: "s1"}) - if err != nil { - return false - } - decoded, err := base64.StdEncoding.DecodeString(encoded) - if err != nil { - return false - } - at := strings.LastIndex(string(decoded), "HOME: ") + decoded := replayOf(t, svc, "s1") + at := strings.LastIndex(decoded, "HOME: ") if at < 0 { return false } - answer = string(decoded)[at:] + answer = decoded[at:] return strings.Contains(answer, "MEHO") }, "a confined session to answer at its prompt") diff --git a/internal/terminal/start.go b/internal/terminal/start.go index e7eeb5854..463ceede0 100644 --- a/internal/terminal/start.go +++ b/internal/terminal/start.go @@ -183,6 +183,12 @@ func (s *Service) spawnSession(opts api.StartOptions, profileDir, name string) ( return } s.emitData(opts.ID, data) + }, func(tail []byte) { + // The /events bridge has no replay: the window that asked for this + // one reads /ws, and without /ws it reads no output at all. + if s.ws != nil { + s.ws.sendReplay(opts.ID, tail) + } }, outboxDepth) out := newCoalescer(box.push, visibleFlushInterval, hiddenFlushInterval) sess := &session{ diff --git a/internal/terminal/terminal.go b/internal/terminal/terminal.go index cda6c6801..f46d05512 100644 --- a/internal/terminal/terminal.go +++ b/internal/terminal/terminal.go @@ -73,6 +73,10 @@ type session struct { done chan struct{} replay *replayBuffer outbox *outbox + // feed holds a PTY read between its append to replay and its write to + // out, so a Replay never takes a tail that holds a read out has yet to + // emit. + feed sync.Mutex // lastOut is when this PTY last produced output, and ready records that it // has since gone quiet once — that its program finished drawing itself and // is waiting on input. Both guarded by the service's mu. diff --git a/internal/terminal/transport.go b/internal/terminal/transport.go index 5ba04fcc5..d1344744b 100644 --- a/internal/terminal/transport.go +++ b/internal/terminal/transport.go @@ -33,6 +33,9 @@ import ( // degraded but unbroken. // // Frame format, both directions: [1 byte id length][session id][payload]. +// A replay frame, backend to window only, is a zero byte and then the same: +// [0][1 byte id length][session id][the session's replayed tail] +// (Service.Replay). const ( // wsWriteTimeout bounds a send to the local client; a stalled write drops @@ -92,6 +95,15 @@ func encodeFrame(id string, payload []byte) ([]byte, error) { return buf, nil } +// encodeReplayFrame is encodeFrame marked as a replay frame. +func encodeReplayFrame(id string, tail []byte) ([]byte, error) { + frame, err := encodeFrame(id, tail) + if err != nil { + return nil, err + } + return append([]byte{0}, frame...), nil +} + // decodeFrame splits a frame into session id and payload. The payload aliases // buf; callers must not retain it past the next read. func decodeFrame(buf []byte) (string, []byte, error) { @@ -781,6 +793,23 @@ func (t *transport) send(id string, data []byte) bool { return out.push(frame) } +// sendReplay queues session id's replay frame to the connected client. With +// no client, or one whose socket refuses it, the replay is dropped rather +// than handed to the /events bridge: decodeFrame does not read it, and the +// bridge carries none. +func (t *transport) sendReplay(id string, tail []byte) { + frame, err := encodeReplayFrame(id, tail) + if err != nil { + return + } + t.mu.Lock() + out := t.out + t.mu.Unlock() + if out != nil { + out.push(frame) + } +} + // flush waits for the queued output of every session to reach the socket. A // session calls it before its exit event so the banner cannot overtake the last // bytes the session wrote. diff --git a/native/backend/api.go b/native/backend/api.go index 014d1dfea..e279e031c 100644 --- a/native/backend/api.go +++ b/native/backend/api.go @@ -2,8 +2,6 @@ package backend import ( "context" - "encoding/base64" - "fmt" "github.com/lichdotdev/lich/api" ) @@ -37,18 +35,12 @@ func (c *Client) SetActiveSession(ctx context.Context, projectID, sessionID stri return err } -// Replay returns the tail of a session's output, to write into a fresh -// terminal before live output is applied. -func (c *Client) Replay(ctx context.Context, id string) ([]byte, error) { - b64, err := callMethod(ctx, c, api.TerminalReplay, api.SessionIDOptions{ID: id}) - if err != nil { - return nil, err - } - data, err := base64.StdEncoding.DecodeString(b64) - if err != nil { - return nil, fmt.Errorf("terminal.Replay: %w", err) - } - return data, nil +// Replay asks for the tail of a session's output, which arrives on the +// Terminal stream as a replay frame: the output frames before it are in the +// tail and the ones after it are not. It reports whether one is coming, false +// for a session not running, which has no output yet. +func (c *Client) Replay(ctx context.Context, id string) (bool, error) { + return callMethod(ctx, c, api.TerminalReplay, api.SessionIDOptions{ID: id}) } type ConversationReported = api.SessionConversation diff --git a/native/backend/client.go b/native/backend/client.go index eba730e52..c85fd0323 100644 --- a/native/backend/client.go +++ b/native/backend/client.go @@ -233,10 +233,11 @@ type Stream struct { mu sync.Mutex } -// Terminal connects to /ws and calls onOutput, from its own goroutine, with -// every output frame until ctx ends or the connection drops; done is closed +// Terminal connects to /ws and calls onOutput with every output frame and +// onReplay with every replay frame (Replay), from its own goroutine and in the +// order they arrive, until ctx ends or the connection drops; done is closed // then. -func (c *Client) Terminal(ctx context.Context, onOutput func(id string, data []byte)) (s *Stream, done <-chan struct{}, err error) { +func (c *Client) Terminal(ctx context.Context, onOutput, onReplay func(id string, data []byte)) (s *Stream, done <-chan struct{}, err error) { conn, _, err := websocket.Dial(ctx, c.url("ws", "/ws"), nil) if err != nil { return nil, nil, fmt.Errorf("terminal stream: %w", err) @@ -254,11 +255,15 @@ func (c *Client) Terminal(ctx context.Context, onOutput func(id string, data []b if typ != websocket.MessageBinary { continue } + handle := onOutput + if len(data) > 1 && data[0] == 0 { + handle, data = onReplay, data[1:] + } id, payload, err := decodeFrame(data) if err != nil { continue } - onOutput(id, payload) + handle(id, payload) } }() return &Stream{conn: conn}, finished, nil @@ -275,7 +280,8 @@ func (s *Stream) Send(ctx context.Context, id string, data []byte) error { return s.conn.Write(ctx, websocket.MessageBinary, frame) } -// A /ws frame is [id length: 1 byte][id][payload] (internal/terminal/transport.go). +// A /ws frame is [id length: 1 byte][id][payload], and a replay frame the same +// behind a zero byte (internal/terminal/transport.go). func encodeFrame(id string, payload []byte) ([]byte, error) { if len(id) == 0 || len(id) > 255 { return nil, fmt.Errorf("session id %q: want 1 to 255 bytes", id) diff --git a/native/backend/live_test.go b/native/backend/live_test.go index a2bb7342f..39e76c3d9 100644 --- a/native/backend/live_test.go +++ b/native/backend/live_test.go @@ -38,7 +38,7 @@ func TestLiveShellSession(t *testing.T) { out.Write(data) mu.Unlock() } - }) + }, func(string, []byte) {}) if err != nil { t.Fatal(err) } diff --git a/native/cmd/lich-native/main.go b/native/cmd/lich-native/main.go index 2c67de508..8efc077ed 100644 --- a/native/cmd/lich-native/main.go +++ b/native/cmd/lich-native/main.go @@ -348,7 +348,7 @@ func (u *window) connect() error { if err != nil { return err } - stream, _, err := u.client.Terminal(u.ctx, func(id string, data []byte) { u.terms.onOutput(id, data) }) + stream, _, err := u.client.Terminal(u.ctx, u.terms.onOutput, u.terms.onReplay) if err != nil { return err } diff --git a/native/cmd/lich-native/terminals.go b/native/cmd/lich-native/terminals.go index 91885871b..c065ca2cb 100644 --- a/native/cmd/lich-native/terminals.go +++ b/native/cmd/lich-native/terminals.go @@ -9,6 +9,7 @@ import ( "runtime" "slices" "sync" + "sync/atomic" "time" "gioui.org/f32" @@ -37,6 +38,9 @@ type termView struct { vt *gioterm.Terminal r gioterm.Renderer tag *int + // seeded is set once vt holds the session's replayed tail: the stream + // writes output into it from then on, and its replies reach the session. + seeded atomic.Bool // composition holds what an input method has not committed yet. composition gioterm.Composition // The area size waiting to be fitted, and since when it has held. @@ -170,7 +174,7 @@ func (ts *terminals) onOutput(id string, data []byte) { ts.mu.Lock() t, shown := ts.byID[id], ts.shown[id] ts.mu.Unlock() - if t == nil { + if t == nil || !t.seeded.Load() { return } t.vt.Write(data) @@ -179,6 +183,21 @@ func (ts *terminals) onOutput(id string, data []byte) { } } +// onReplay seeds a terminal attach left waiting with its session's tail. +func (ts *terminals) onReplay(id string, tail []byte) { + ts.mu.Lock() + t, shown := ts.byID[id], ts.shown[id] + ts.mu.Unlock() + if t == nil || t.seeded.Load() { + return + } + t.vt.Write(tail) + t.seeded.Store(true) + if shown { + ts.invalidate() + } +} + // openLink opens url in the user's browser, off the frame. func (ts *terminals) openLink(url string) { go func() { @@ -209,9 +228,7 @@ func (ts *terminals) has(id string) bool { // show returns s's terminal, attaching it on first use: the replayed tail // first, then Start (a no-op for a running session) resuming the conversation -// resume names, "" for none, then the size. ponytail: output that lands between -// Replay and registering the view is dropped; subscribing before replaying -// would keep it. +// resume names, "" for none, then the size. func (ts *terminals) show(ctx context.Context, gtx layout.Context, s sessionView, resume string) (*termView, error) { ts.mu.Lock() t := ts.byID[s.ID] @@ -231,28 +248,10 @@ func (ts *terminals) show(ctx context.Context, gtx layout.Context, s sessionView return nil, err } cols, rows := gioterm.Fit(gridArea(gtx), r) - cellW, cellH := r.Cell() - vt, err := gioterm.New(cols, rows, cellW, cellH, func(b []byte) { ts.send(s.ID, b) }) - if err != nil { + t = &termView{id: s.ID, kind: s.Kind, r: r, tag: new(int)} + if err := ts.attach(ctx, t, cols, rows); err != nil { return nil, err } - ts.mu.Lock() - colors := ts.colors - ts.mu.Unlock() - if err := vt.SetColors(colors); err != nil { - return nil, err - } - replay, err := ts.client.Replay(ctx, s.ID) - if err != nil { - return nil, err - } - if len(replay) > 0 { - vt.Write(replay) - } - t = &termView{id: s.ID, kind: s.Kind, vt: vt, r: r, tag: new(int)} - ts.mu.Lock() - ts.byID[s.ID] = t - ts.mu.Unlock() // A failed start leaves the terminal in place, empty. if err := ts.start(ctx, s, resume, cols, rows); err != nil { if backend.ProfileGone(err) && ts.profileGone != nil { @@ -270,6 +269,43 @@ func (ts *terminals) show(ctx context.Context, gtx layout.Context, s sessionView return t, ts.client.Resize(ctx, s.ID, cols, rows) } +// attach gives t a cols x rows terminal, registers it and asks for the +// session's replayed tail. Until the tail arrives (onReplay) the terminal +// takes no output, which the tail already holds, and sends no replies: what +// it answers while it replays are replies to queries the program already had +// answered, and sent now they would land on its input as typed text. +func (ts *terminals) attach(ctx context.Context, t *termView, cols, rows int) error { + cellW, cellH := t.r.Cell() + vt, err := gioterm.New(cols, rows, cellW, cellH, func(b []byte) { + if t.seeded.Load() { + ts.send(t.id, b) + } + }) + if err != nil { + return err + } + ts.mu.Lock() + colors := ts.colors + ts.mu.Unlock() + if err := vt.SetColors(colors); err != nil { + vt.Close() + return err + } + t.vt = vt + ts.mu.Lock() + ts.byID[t.id] = t + ts.mu.Unlock() + coming, err := ts.client.Replay(ctx, t.id) + if err != nil { + ts.drop(t.id) + return err + } + if !coming { + t.seeded.Store(true) + } + return nil +} + // setColors gives every terminal, and every one opened after, a theme's // default colors. The lock is released before painting: a terminal whose // scheme flips under mode 2031 reports it through send, which takes the lock. diff --git a/native/cmd/lich-native/terminals_colors_test.go b/native/cmd/lich-native/terminals_colors_test.go index 6e61c667a..3f81b1ec4 100644 --- a/native/cmd/lich-native/terminals_colors_test.go +++ b/native/cmd/lich-native/terminals_colors_test.go @@ -32,7 +32,7 @@ func TestSchemeFlippingThemeReportsToTheSession(t *testing.T) { })) t.Cleanup(srv.Close) client := clientOf(srv) - stream, _, err := client.Terminal(t.Context(), func(string, []byte) {}) + stream, _, err := client.Terminal(t.Context(), func(string, []byte) {}, func(string, []byte) {}) if err != nil { t.Fatal(err) } diff --git a/native/cmd/lich-native/terminals_replay_test.go b/native/cmd/lich-native/terminals_replay_test.go new file mode 100644 index 000000000..81f11acb3 --- /dev/null +++ b/native/cmd/lich-native/terminals_replay_test.go @@ -0,0 +1,64 @@ +package main + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/coder/websocket" +) + +// A terminal attached to a running session takes nothing from the stream +// until its replay frame: output before it is already in the tail. The +// program's queries in the tail were answered when the program asked them, +// so reopening the window must not type the answers into the session again, +// while a query after the replay still gets its answer. +func TestAttachedTerminalStartsFromTheReplay(t *testing.T) { + frames := make(chan []byte, 4) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasPrefix(r.URL.Path, "/rpc/") { + _ = json.NewEncoder(w).Encode(true) + return + } + conn, err := websocket.Accept(w, r, nil) + if err != nil { + return + } + defer conn.CloseNow() + for { + _, data, err := conn.Read(r.Context()) + if err != nil { + return + } + frames <- data + } + })) + t.Cleanup(srv.Close) + client := clientOf(srv) + stream, _, err := client.Terminal(t.Context(), func(string, []byte) {}, func(string, []byte) {}) + if err != nil { + t.Fatal(err) + } + ts := &terminals{client: client, stream: stream, byID: map[string]*termView{}, invalidate: func() {}} + tv := &termView{id: "s", r: fixedCells{}} + if err := ts.attach(t.Context(), tv, 80, 24); err != nil { + t.Fatal(err) + } + t.Cleanup(tv.vt.Close) + + ts.onOutput("s", []byte("already in the tail\r\n\r\n\r\n")) + ts.onReplay("s", []byte("$ nvim\r\n\x1b[c\x1b[6n")) + ts.onOutput("s", []byte("\x1b[6n")) + + select { + case got := <-frames: + if want := "\x01s\x1b[2;1R"; string(got) != want { + t.Fatalf("session got %q, want only the live cursor report %q", got, want) + } + case <-time.After(5 * time.Second): + t.Fatal("the query after the replay was never answered") + } +} From b249c33b86a0cd7189b89fb9a55c0188d6e20fbb Mon Sep 17 00:00:00 2001 From: omartelo Date: Fri, 23 Oct 2026 16:54:42 -0300 Subject: [PATCH 2/4] perf(fonts): ask fontconfig once per face for the rest of the run Every fc-match is a subprocess, and a terminal asked for its family's four styles each time its renderer was built: a card's first show and every font size step, inside the frame. Locate keeps each answer by the font asked for. A family installed after it was asked for keeps its substitute until lich restarts, written down in docs/ceilings.md. --- CHANGELOG.md | 1 + docs/ceilings.md | 4 ++++ native/fonts/locate_fontconfig.go | 23 ++++++++++++++++++++-- native/fonts/locate_fontconfig_test.go | 27 ++++++++++++++++++++++++++ 4 files changed, 53 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 250134f01..f3e6bb70f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Expanding the sidebar no longer leaves the session's terminal at its old width for a beat once the sidebar is open. The terminal takes its new width as the sidebar starts moving, as with the dock, where it waited until a tenth of a second after the sidebar stopped and then jumped; collapsing it refits at once too. A split layout's terminals fit the moment the sidebar stops. - Reopening the window no longer types stray text into a session. Rebuilding a terminal from its recent output answered the terminal queries in it again, such as the ones nvim sends at start, and the answers landed on the session's input: in a shell as a `command not found`, in an agent as text in its prompt. - A session's terminal opened while the session is printing no longer misses what it printed in that moment. The backend now hands the window the session's recent output in line with its live output, so nothing between the two is lost or shown twice. +- On Linux, stepping the terminal font size and opening a card for the first time hitch less. Every terminal looked its font up again through fontconfig each time, four lookups of about 10ms apiece inside one frame; lich now remembers the answer for the rest of its run. ## [0.78.0] - 2026-10-22 diff --git a/docs/ceilings.md b/docs/ceilings.md index d71dab17c..e49c0243e 100644 --- a/docs/ceilings.md +++ b/docs/ceilings.md @@ -897,6 +897,10 @@ work when nobody knows it and that the call site never shows. The mechanism and and macOS leave out the dual-width Nerd Font families (the ones not named "Mono", whose icons take two cells) that fontconfig's `spacing=dual` lists on Linux; the "Mono" variant of each is offered. Its family names come from parsing one file per family, a few hundred on a stock Windows, the first time a font picker opens. + On Linux each face fontconfig names is kept for the rest of the run (`fonts.Locate`), since every lookup is an + `fc-match` subprocess inside a frame: a family installed while lich runs, after a terminal or the editor + already asked for it, keeps drawing in the substitute fontconfig gave until lich restarts. Choosing it in a + font picker for the first time is not affected. - **The Chinese font the native window downloads reaches its interface, not its terminal on Linux** (`native/fonts/download.go`, `native/glyphs/raster.go`): the terminal finds a glyph its family lacks through fontconfig (`fonts.Covering`), which never sees `/lich/fonts`, so with no Chinese font diff --git a/native/fonts/locate_fontconfig.go b/native/fonts/locate_fontconfig.go index 1a3eb19d8..62fc69dce 100644 --- a/native/fonts/locate_fontconfig.go +++ b/native/fonts/locate_fontconfig.go @@ -22,10 +22,29 @@ var fontconfigWeights = map[font.Weight]int{ font.Bold: 200, } +// located is every face Locate found, by the font it was asked for: each +// ask is an fc-match subprocess, and a terminal asks for its family's four +// styles whenever its renderer is rebuilt, a font size step included. A +// family installed after it was asked for keeps its substitute until lich +// restarts. +var located sync.Map + // Locate asks fontconfig for the face that draws f's family in its weight -// and style. fontconfig answers a weight or style the family lacks, or a -// family not installed, with its nearest. +// and style, once per process for each f. fontconfig answers a weight or +// style the family lacks, or a family not installed, with its nearest. func Locate(f font.Font) (Location, error) { + if loc, ok := located.Load(f); ok { + return loc.(Location), nil + } + loc, err := match(f) + if err != nil { + return Location{}, err + } + located.Store(f, loc) + return loc, nil +} + +func match(f font.Font) (Location, error) { weight, ok := fontconfigWeights[f.Weight] if !ok { return Location{}, fmt.Errorf("locate %s: no fontconfig weight for %v", f.Typeface, f.Weight) diff --git a/native/fonts/locate_fontconfig_test.go b/native/fonts/locate_fontconfig_test.go index dd4e71aae..6baf4ea4d 100644 --- a/native/fonts/locate_fontconfig_test.go +++ b/native/fonts/locate_fontconfig_test.go @@ -9,6 +9,8 @@ import ( "slices" "strings" "testing" + + "gioui.org/font" ) func TestParseFcListFiles(t *testing.T) { @@ -55,6 +57,31 @@ func TestCoveringRanksAgainstMonospaceOnce(t *testing.T) { } } +// A face fontconfig already named is the same answer the next time a +// terminal's renderer is built, and asking again is another subprocess +// inside a frame. +func TestLocateAsksFontconfigOncePerFont(t *testing.T) { + bold := font.Font{Typeface: SystemMono, Weight: font.Bold} + first, err := Locate(bold) + if err != nil { + t.Fatal(err) + } + calls := countCalls(t, "fc-match") + again, err := Locate(bold) + if err != nil || again != first { + t.Fatalf("Locate again = %+v, %v; want %+v", again, err, first) + } + if got := calls(); got != 0 { + t.Fatalf("a second Locate ran fc-match %d times, want 0", got) + } + if _, err := Locate(font.Font{Typeface: SystemMono, Weight: font.Bold, Style: font.Italic}); err != nil { + t.Fatal(err) + } + if got := calls(); got != 1 { + t.Fatalf("Locate of another style ran fc-match %d times, want 1", got) + } +} + // countCalls puts a wrapper of the real command first on PATH and returns // how many times it has run since. func countCalls(t *testing.T, command string) func() int { From dc3ff48ffff339ced2dddc02736319964e1f1f3c Mon Sep 17 00:00:00 2001 From: omartelo Date: Fri, 23 Oct 2026 16:54:42 -0300 Subject: [PATCH 3/4] perf(native): resize the terminals' renderers on a font size step A size step built a new renderer for every open terminal, loading the family's faces again inside the frame. Renderer gains SetSize, which the next Measure takes in the same way it takes a new display scale; a new family still builds a renderer. --- CHANGELOG.md | 1 + native/cmd/lich-native/terminal_hit_test.go | 1 + native/cmd/lich-native/terminals.go | 9 ++++-- native/cmd/lich-native/terminals_font_test.go | 31 +++++++++++++++++++ native/gioterm/render.go | 2 ++ native/gioterm/render_atlas.go | 2 ++ native/gioterm/render_paths.go | 2 ++ native/gioterm/render_rows.go | 2 ++ 8 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 native/cmd/lich-native/terminals_font_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index f3e6bb70f..69f8aabeb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Reopening the window no longer types stray text into a session. Rebuilding a terminal from its recent output answered the terminal queries in it again, such as the ones nvim sends at start, and the answers landed on the session's input: in a shell as a `command not found`, in an agent as text in its prompt. - A session's terminal opened while the session is printing no longer misses what it printed in that moment. The backend now hands the window the session's recent output in line with its live output, so nothing between the two is lost or shown twice. - On Linux, stepping the terminal font size and opening a card for the first time hitch less. Every terminal looked its font up again through fontconfig each time, four lookups of about 10ms apiece inside one frame; lich now remembers the answer for the rest of its run. +- Stepping the terminal font size no longer reloads the font of every open terminal: each one keeps the font it has and only draws it at the new size. ## [0.78.0] - 2026-10-22 diff --git a/native/cmd/lich-native/terminal_hit_test.go b/native/cmd/lich-native/terminal_hit_test.go index 2c89eaa43..b34b460d4 100644 --- a/native/cmd/lich-native/terminal_hit_test.go +++ b/native/cmd/lich-native/terminal_hit_test.go @@ -23,6 +23,7 @@ func (fixedCells) Cell() (int, int) { return mouseTestCellW, mouseTe func (fixedCells) Draw(layout.Context, gioterm.Snapshot) (int, error) { return 0, nil } +func (fixedCells) SetSize(unit.Sp) {} // A pointer over a cell of the laid-out terminal selects that cell, wherever // the padding puts the grid in the window. diff --git a/native/cmd/lich-native/terminals.go b/native/cmd/lich-native/terminals.go index c065ca2cb..8ce2acacc 100644 --- a/native/cmd/lich-native/terminals.go +++ b/native/cmd/lich-native/terminals.go @@ -322,8 +322,9 @@ func (ts *terminals) setColors(c gioterm.Colors) { } // setFont draws every terminal, and every one opened after, in family at -// size. Each gets a renderer of the new face; its next frame measures the -// cells and refits the grid and the PTY to them. +// size. A new family gives each a renderer of the new face, a new size only +// resizes the one it has; its next frame measures the cells and refits the +// grid and the PTY to them. func (ts *terminals) setFont(family string, size unit.Sp) error { ts.mu.Lock() defer ts.mu.Unlock() @@ -331,6 +332,10 @@ func (ts *terminals) setFont(family string, size unit.Sp) error { return nil } for _, t := range ts.byID { + if family == ts.family { + t.r.SetSize(size) + continue + } r, err := gioterm.NewRenderer(ts.renderer, family, size) if err != nil { return err diff --git a/native/cmd/lich-native/terminals_font_test.go b/native/cmd/lich-native/terminals_font_test.go new file mode 100644 index 000000000..f161cad5d --- /dev/null +++ b/native/cmd/lich-native/terminals_font_test.go @@ -0,0 +1,31 @@ +package main + +import ( + "testing" + + "gioui.org/unit" +) + +type sizedRenderer struct { + fixedCells + size unit.Sp +} + +func (r *sizedRenderer) SetSize(size unit.Sp) { r.size = size } + +// A font size step keeps each terminal's renderer, faces and all, and only +// resizes it: building a new one loads the family's faces again inside the +// frame the step lands in. +func TestFontSizeStepResizesTheRenderersItHas(t *testing.T) { + r := &sizedRenderer{size: 13} + ts := &terminals{family: "Mono", size: 13, byID: map[string]*termView{"s": {id: "s", r: r}}, invalidate: func() {}} + if err := ts.setFont("Mono", 15); err != nil { + t.Fatal(err) + } + if ts.byID["s"].r != r { + t.Fatal("a size step replaced the terminal's renderer") + } + if r.size != 15 || ts.size != 15 { + t.Fatalf("renderer at %v, terminals at %v; want both at 15", r.size, ts.size) + } +} diff --git a/native/gioterm/render.go b/native/gioterm/render.go index ec762abd8..d22a1392f 100644 --- a/native/gioterm/render.go +++ b/native/gioterm/render.go @@ -23,6 +23,8 @@ type Renderer interface { Measure(gtx layout.Context) error // Cell is the size of one cell in pixels. Cell() (w, h int) + // SetSize changes the font size, which the next Measure takes in. + SetSize(size unit.Sp) // Draw paints the snapshot at the current offset and returns how many rows // it had to redraw. Draw(gtx layout.Context, s Snapshot) (int, error) diff --git a/native/gioterm/render_atlas.go b/native/gioterm/render_atlas.go index 8b4238693..8aeee5622 100644 --- a/native/gioterm/render_atlas.go +++ b/native/gioterm/render_atlas.go @@ -59,6 +59,8 @@ func newAtlasRenderer(r *glyphs.Rasterizer, size unit.Sp) *atlasRenderer { func (a *atlasRenderer) Cell() (w, h int) { return a.r.CellW(), a.r.CellH() } +func (a *atlasRenderer) SetSize(size unit.Sp) { a.size = size } + func (a *atlasRenderer) Measure(gtx layout.Context) error { changed, err := a.r.SetSize(float64(gtx.Sp(a.size))) if changed { diff --git a/native/gioterm/render_paths.go b/native/gioterm/render_paths.go index 2e1bb178a..0695d1902 100644 --- a/native/gioterm/render_paths.go +++ b/native/gioterm/render_paths.go @@ -59,6 +59,8 @@ func loadFamily(family string) ([]font.FontFace, error) { func (g *grid) Cell() (w, h int) { return g.cellW, g.cellH } +func (g *grid) SetSize(size unit.Sp) { g.size = size } + func (g *grid) Measure(gtx layout.Context) error { px := fixed.I(gtx.Sp(g.size)) if px == g.pxPerEm { diff --git a/native/gioterm/render_rows.go b/native/gioterm/render_rows.go index abe64655f..30f5caf00 100644 --- a/native/gioterm/render_rows.go +++ b/native/gioterm/render_rows.go @@ -31,6 +31,8 @@ func newRowRenderer(r *glyphs.Rasterizer, size unit.Sp) *rowRenderer { func (rr *rowRenderer) Cell() (w, h int) { return rr.r.CellW(), rr.r.CellH() } +func (rr *rowRenderer) SetSize(size unit.Sp) { rr.size = size } + func (rr *rowRenderer) Measure(gtx layout.Context) error { changed, err := rr.r.SetSize(float64(gtx.Sp(rr.size))) if changed { From fdba2679a1bf85445b7f0b256a2c7923fd958ca6 Mon Sep 17 00:00:00 2001 From: omartelo Date: Fri, 23 Oct 2026 17:25:27 -0300 Subject: [PATCH 4/4] perf(terminal): copy a session's output once on its way to the window A hidden session flooding at ~147 MB/s made the backend allocate about 7.7x its output: the coalescer dropped its batch buffer after every flush and grew a new one, the replay tail allocated a chunk per PTY read, and the outbox copied each batch only for the transport to copy it again into the /ws frame. - The coalescer keeps its batch buffer across flushes; emit already may not retain it. - The replay tail is one buffer grown up to its 2 MB cap and then overwritten from its oldest byte. - The outbox copies a batch straight into its /ws frame behind the session's header, and the transport sends that frame as is. Start refuses a session id no frame header can carry (over 255 bytes) before its PTY spawns, where its output used to fall back to /events, which the window does not read. BenchmarkHiddenFlood (new): 232 KB allocated per 32 KB read, 2 allocs, down to 34 KB and 0 allocs. TestReplayBufferKeepsOneChunkLargerThanCap is replaced by TestReplayBufferKeepsTheEndOfAWriteLargerThanCap: the contract changed, a write larger than the cap now keeps its last capBytes (a PTY read never is one). The transport tests build frames with frameFor, since send takes a frame. --- CHANGELOG.md | 1 + internal/terminal/coalescer.go | 4 +- internal/terminal/feed_bench_test.go | 33 +++++++++++++ internal/terminal/outbox.go | 22 ++++++--- internal/terminal/outbox_test.go | 6 +-- internal/terminal/replay.go | 53 +++++++++++---------- internal/terminal/replay_test.go | 32 ++++++++++--- internal/terminal/respawn_test.go | 14 ++++++ internal/terminal/start.go | 10 ++-- internal/terminal/transport.go | 8 +--- internal/terminal/transport_replace_test.go | 2 +- internal/terminal/transport_stall_test.go | 6 +-- internal/terminal/transport_test.go | 4 +- internal/terminal/writequeue_test.go | 2 +- 14 files changed, 137 insertions(+), 60 deletions(-) create mode 100644 internal/terminal/feed_bench_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 69f8aabeb..c180257f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - A session's terminal opened while the session is printing no longer misses what it printed in that moment. The backend now hands the window the session's recent output in line with its live output, so nothing between the two is lost or shown twice. - On Linux, stepping the terminal font size and opening a card for the first time hitch less. Every terminal looked its font up again through fontconfig each time, four lookups of about 10ms apiece inside one frame; lich now remembers the answer for the rest of its run. - Stepping the terminal font size no longer reloads the font of every open terminal: each one keeps the font it has and only draws it at the new size. +- A session printing a lot of output costs the backend less. Each byte on its way to the window was copied about seven times into memory the garbage collector then had to reclaim; it is now copied once. ## [0.78.0] - 2026-10-22 diff --git a/internal/terminal/coalescer.go b/internal/terminal/coalescer.go index 2051c9e01..15f608924 100644 --- a/internal/terminal/coalescer.go +++ b/internal/terminal/coalescer.go @@ -119,7 +119,7 @@ func (c *coalescer) cut(then func()) { c.timer.Stop() c.timer = nil } - c.pending = nil + c.pending = c.pending[:0] then() } @@ -133,7 +133,7 @@ func (c *coalescer) flushLocked() { return } c.emit(c.pending) - c.pending = nil + c.pending = c.pending[:0] c.lastEmit = time.Now() } diff --git a/internal/terminal/feed_bench_test.go b/internal/terminal/feed_bench_test.go new file mode 100644 index 000000000..837a19d4c --- /dev/null +++ b/internal/terminal/feed_bench_test.go @@ -0,0 +1,33 @@ +package terminal + +import ( + "testing" + "time" +) + +// BenchmarkHiddenFlood feeds PTY reads through a hidden session's output path +// the way stream does (replay tail, coalescer, outbox, the /ws frame) and +// reports what that path allocates per byte of output. +func BenchmarkHiddenFlood(b *testing.B) { + read := make([]byte, readBufSize) + for i := range read { + read[i] = 'x' + } + replay := newReplayBuffer(replayCapBytes) + header, err := encodeFrame("session-id", nil) + if err != nil { + b.Fatal(err) + } + box := newOutbox(header, func([]byte) {}, func([]byte) {}, outboxDepth) + out := newCoalescer(box.push, visibleFlushInterval, time.Hour) + out.SetVisible(false) + b.SetBytes(int64(len(read))) + b.ReportAllocs() + b.ResetTimer() + for range b.N { + replay.append(read) + out.Write(read) + } + out.Close() + box.close() +} diff --git a/internal/terminal/outbox.go b/internal/terminal/outbox.go index 3ffc57d60..278079279 100644 --- a/internal/terminal/outbox.go +++ b/internal/terminal/outbox.go @@ -20,6 +20,8 @@ const outboxDepth = 32 type outbox struct { frames chan outFrame done chan struct{} + // prefix heads every output frame: the /ws frame header of its session. + prefix []byte } type outFrame struct { @@ -28,10 +30,11 @@ type outFrame struct { } // newOutbox starts the delivery goroutine. deliver is called with each output -// frame and replay with each replay frame, one at a time and in the order they -// were queued; both own the slice they are handed. -func newOutbox(deliver, replay func(data []byte), depth int) *outbox { - o := &outbox{frames: make(chan outFrame, depth), done: make(chan struct{})} +// frame, the pushed output behind prefix, and replay with each replay frame's +// tail, one at a time and in the order they were queued; both own the slice +// they are handed. +func newOutbox(prefix []byte, deliver, replay func(data []byte), depth int) *outbox { + o := &outbox{frames: make(chan outFrame, depth), done: make(chan struct{}), prefix: prefix} go func() { defer close(o.done) for frame := range o.frames { @@ -45,10 +48,15 @@ func newOutbox(deliver, replay func(data []byte), depth int) *outbox { return o } -// push queues one frame for delivery, blocking only while this session's queue -// is full. The frame is copied: the coalescer reuses the buffer it passes. +// push queues data behind the prefix for delivery, blocking only while this +// session's queue is full. data is copied into the frame, which is the one +// copy output takes on its way to the socket: the coalescer reuses the buffer +// it passes. func (o *outbox) push(data []byte) { - o.frames <- outFrame{data: append([]byte(nil), data...)} + frame := make([]byte, len(o.prefix)+len(data)) + copy(frame, o.prefix) + copy(frame[len(o.prefix):], data) + o.frames <- outFrame{data: frame} } // pushReplay queues the session's replayed tail behind every frame already diff --git a/internal/terminal/outbox_test.go b/internal/terminal/outbox_test.go index 4e2150adf..29a73f9bb 100644 --- a/internal/terminal/outbox_test.go +++ b/internal/terminal/outbox_test.go @@ -15,7 +15,7 @@ func TestOutboxQueuesWhileDeliveryIsStuck(t *testing.T) { t.Parallel() release := make(chan struct{}) entered := make(chan struct{}, 1) - box := newOutbox(func([]byte) { + box := newOutbox(nil, func([]byte) { select { case entered <- struct{}{}: default: @@ -64,7 +64,7 @@ func TestOutboxDeliversInOrderAndDrainsOnClose(t *testing.T) { t.Parallel() var mu sync.Mutex var got []string - box := newOutbox(func(data []byte) { + box := newOutbox(nil, func(data []byte) { mu.Lock() got = append(got, string(data)) mu.Unlock() @@ -87,7 +87,7 @@ func TestOutboxDeliversInOrderAndDrainsOnClose(t *testing.T) { func TestOutboxCopiesTheFrame(t *testing.T) { t.Parallel() delivered := make(chan []byte, 1) - box := newOutbox(func(data []byte) { delivered <- data }, nil, 1) + box := newOutbox(nil, func(data []byte) { delivered <- data }, nil, 1) buf := []byte("live") box.push(buf) diff --git a/internal/terminal/replay.go b/internal/terminal/replay.go index 8a0d2709f..b9f280a64 100644 --- a/internal/terminal/replay.go +++ b/internal/terminal/replay.go @@ -11,14 +11,16 @@ const replayCapBytes = 2 << 20 // replayBuffer keeps a capped tail of a session's raw PTY output so a // window attaching to a running session can reseed its scrollback: the PTY // (and this tail) live on in the backend while no window holds the terminal. -// A chunk list dropped from the front on overflow, so after a drop the head -// may be a partial ANSI sequence, an accepted artifact (TUIs repaint, shells -// scroll it away). Safe for concurrent append (the stream goroutine) and -// snapshot (an RPC call). +// One buffer, grown as output arrives up to the cap and then overwritten from +// its oldest byte, so a read costs a copy and no allocation. After the first +// overwrite the head may be a partial ANSI or UTF-8 sequence, an accepted +// artifact (TUIs repaint, shells scroll it away). Safe for concurrent append +// (the stream goroutine) and snapshot (an RPC call). type replayBuffer struct { - mu sync.Mutex - chunks [][]byte - total int + mu sync.Mutex + buf []byte + // oldest is where the tail starts in buf once buf holds capBytes. + oldest int capBytes int } @@ -26,35 +28,36 @@ func newReplayBuffer(capBytes int) *replayBuffer { return &replayBuffer{capBytes: capBytes} } -// append copies data (the caller reuses its read buffer) and enqueues it, -// dropping the oldest chunks while the total exceeds the cap. One chunk is -// always kept, so a single write larger than the cap still replays. +// append copies data (the caller reuses its read buffer) into the tail, +// overwriting the oldest bytes once the tail holds capBytes. A write larger +// than the cap keeps its last capBytes. func (b *replayBuffer) append(data []byte) { if len(data) == 0 { return } - chunk := make([]byte, len(data)) - copy(chunk, data) - b.mu.Lock() defer b.mu.Unlock() - b.chunks = append(b.chunks, chunk) - b.total += len(chunk) - for b.total > b.capBytes && len(b.chunks) > 1 { - b.total -= len(b.chunks[0]) - // Reslicing alone keeps the chunk reachable through the backing array. - b.chunks[0] = nil - b.chunks = b.chunks[1:] + if len(data) > b.capBytes { + data = data[len(data)-b.capBytes:] + } + if room := b.capBytes - len(b.buf); room > 0 { + n := min(room, len(data)) + b.buf = append(b.buf, data[:n]...) + data = data[n:] + } + for len(data) > 0 { + n := copy(b.buf[b.oldest:], data) + b.oldest = (b.oldest + n) % b.capBytes + data = data[n:] } } -// snapshot returns the queued tail as one contiguous slice in arrival order. +// snapshot returns the tail as one contiguous slice in arrival order. func (b *replayBuffer) snapshot() []byte { b.mu.Lock() defer b.mu.Unlock() - out := make([]byte, 0, b.total) - for _, c := range b.chunks { - out = append(out, c...) - } + out := make([]byte, len(b.buf)) + n := copy(out, b.buf[b.oldest:]) + copy(out[n:], b.buf[:b.oldest]) return out } diff --git a/internal/terminal/replay_test.go b/internal/terminal/replay_test.go index 0ff14f8f8..960d7a7e8 100644 --- a/internal/terminal/replay_test.go +++ b/internal/terminal/replay_test.go @@ -1,7 +1,6 @@ package terminal import ( - "bytes" "github.com/lichdotdev/lich/api" "slices" "strings" @@ -55,13 +54,32 @@ func TestReplayBufferDropsOldestOnOverflow(t *testing.T) { } } -func TestReplayBufferKeepsOneChunkLargerThanCap(t *testing.T) { +// The tail is the last capBytes of output, however the reads that brought it +// were cut: a read the cap splits keeps the part that is still in the tail. +func TestReplayBufferKeepsTheLastCapBytesAcrossReads(t *testing.T) { + t.Parallel() + b := newReplayBuffer(10) + for _, read := range []string{"0123456", "789ab", "cdefg"} { + b.append([]byte(read)) + } + if got := string(b.snapshot()); got != "789abcdefg" { + t.Fatalf("snapshot = %q, want %q", got, "789abcdefg") + } + b.append([]byte("h")) + if got := string(b.snapshot()); got != "89abcdefgh" { + t.Fatalf("snapshot after one more byte = %q, want %q", got, "89abcdefgh") + } +} + +// A single write larger than the cap keeps its last capBytes. The PTY's read +// buffer is far below the cap, so a read never is one. +func TestReplayBufferKeepsTheEndOfAWriteLargerThanCap(t *testing.T) { t.Parallel() b := newReplayBuffer(4) - big := bytes.Repeat([]byte("x"), 100) - b.append(big) - if got := b.snapshot(); len(got) != 100 { - t.Fatalf("snapshot len = %d, want 100 — a lone oversized chunk must survive", len(got)) + b.append([]byte("ab")) + b.append([]byte("wxyz12")) + if got := string(b.snapshot()); got != "yz12" { + t.Fatalf("snapshot = %q, want %q", got, "yz12") } } @@ -72,7 +90,7 @@ func TestReplayBufferKeepsOneChunkLargerThanCap(t *testing.T) { func TestServiceReplayQueuesTheTailBetweenOutputFrames(t *testing.T) { t.Parallel() var got []string - box := newOutbox( + box := newOutbox(nil, func(data []byte) { got = append(got, "out:"+string(data)) }, func(tail []byte) { got = append(got, "replay:"+string(tail)) }, outboxDepth) diff --git a/internal/terminal/respawn_test.go b/internal/terminal/respawn_test.go index 55a55384e..f0ab28ecc 100644 --- a/internal/terminal/respawn_test.go +++ b/internal/terminal/respawn_test.go @@ -209,3 +209,17 @@ func TestReapForgetsOpenTurn(t *testing.T) { } waitFor(t, func() bool { return open.Load() == 0 }, "the reap to close the turn") } + +// A session's output reaches the window in /ws frames that carry its id in +// at most 255 bytes, so an id longer than that is refused before its PTY +// spawns rather than spawning a session whose output cannot be addressed. +func TestStartRefusesAnIDNoFrameCanCarry(t *testing.T) { + svc := New(stubBins{bin: echoBin(t)}, nil, events.New()) + id := strings.Repeat("x", 256) + if _, err := svc.Start(t.Context(), api.StartOptions{ID: id, ProjectID: "p1", Cwd: t.TempDir(), Kind: "claude", Cols: 80, Rows: 24}); err == nil { + t.Fatal("Start accepted a 256-byte session id") + } + if replayOf(t, svc, id) != "" || svc.ptyOf(id) != nil { + t.Fatal("a refused start left a session behind") + } +} diff --git a/internal/terminal/start.go b/internal/terminal/start.go index 463ceede0..97ca16512 100644 --- a/internal/terminal/start.go +++ b/internal/terminal/start.go @@ -173,16 +173,20 @@ func (s *Service) spawnSession(opts api.StartOptions, profileDir, name string) ( spec, skippedLinks := wrapSandbox( spec, opts.Kind, userHome(), sessionDropDir(s.dropDir, opts.ID, inSandbox), profileDir, inSandbox, creds, ) + header, err := encodeFrame(opts.ID, nil) + if err != nil { + return nil, "", fmt.Errorf("start %q: %w", opts.ID, err) + } p, err := startPTY(spec) if err != nil { return nil, "", fmt.Errorf("failed to start pty for %q: %w", opts.ID, err) } - box := newOutbox(func(data []byte) { - if s.ws != nil && s.ws.send(opts.ID, data) { + box := newOutbox(header, func(frame []byte) { + if s.ws != nil && s.ws.send(frame) { return } - s.emitData(opts.ID, data) + s.emitData(opts.ID, frame[len(header):]) }, func(tail []byte) { // The /events bridge has no replay: the window that asked for this // one reads /ws, and without /ws it reads no output at all. diff --git a/internal/terminal/transport.go b/internal/terminal/transport.go index d1344744b..2ad42e25b 100644 --- a/internal/terminal/transport.go +++ b/internal/terminal/transport.go @@ -771,7 +771,7 @@ func (t *transport) drop(conn *websocket.Conn) { // send hands one session's output frame to the connected client's writer. It // returns false when the caller should fall back to the event bridge itself: -// there is no client, or the frame cannot be addressed to one. A frame the +// there is no client. A frame the // writer accepts and the socket then refuses is not the caller's problem — the // writer takes it to the same bridge, in order (see writequeue.go). // @@ -779,11 +779,7 @@ func (t *transport) drop(conn *websocket.Conn) { // by wsWriteTimeout, and every session's outbox goroutine comes through here; // holding a lock across it would recouple the sessions the outbox queues exist // to keep apart. -func (t *transport) send(id string, data []byte) bool { - frame, err := encodeFrame(id, data) - if err != nil { - return false - } +func (t *transport) send(frame []byte) bool { t.mu.Lock() out := t.out t.mu.Unlock() diff --git a/internal/terminal/transport_replace_test.go b/internal/terminal/transport_replace_test.go index a033847b0..cc979aaa4 100644 --- a/internal/terminal/transport_replace_test.go +++ b/internal/terminal/transport_replace_test.go @@ -52,7 +52,7 @@ func TestReplacedClientDoesNotSilenceTransport(t *testing.T) { // second call is the no-op the first one already was. tr.drop(replaced) - if !tr.send("sess", []byte("output")) { + if !tr.send(frameFor(t, "sess", "output")) { t.Fatal("send reported no client after the replaced one was dropped") } // Its own deadline: sharing the one above would let a slow swap eat this diff --git a/internal/terminal/transport_stall_test.go b/internal/terminal/transport_stall_test.go index 17128fe7c..73731f655 100644 --- a/internal/terminal/transport_stall_test.go +++ b/internal/terminal/transport_stall_test.go @@ -54,9 +54,9 @@ func TestStalledSessionDoesNotBlockAnother(t *testing.T) { loud := make(chan struct{}) go func() { defer close(loud) - payload := make([]byte, stallFrameBytes) + frame := frameFor(t, "loud", string(make([]byte, stallFrameBytes))) for i := 0; i < stallFrames; i++ { - tr.send("loud", payload) + tr.send(frame) accepted.Add(1) } }() @@ -64,7 +64,7 @@ func TestStalledSessionDoesNotBlockAnother(t *testing.T) { waitForStall(t, &accepted, loud) start := time.Now() - tr.send("quiet", []byte("still here")) + tr.send(frameFor(t, "quiet", "still here")) blocked := time.Since(start) t.Logf("a send for an idle session waited %v on a stalled one", blocked.Round(time.Millisecond)) if blocked > stallBudget { diff --git a/internal/terminal/transport_test.go b/internal/terminal/transport_test.go index bb154b3ac..50b52d969 100644 --- a/internal/terminal/transport_test.go +++ b/internal/terminal/transport_test.go @@ -163,7 +163,7 @@ func sendWhenRegistered(t *testing.T, tr *transport, id string, data []byte) boo t.Helper() deadline := time.Now().Add(5 * time.Second) for time.Now().Before(deadline) { - if tr.send(id, data) { + if tr.send(frameFor(t, id, string(data))) { return true } time.Sleep(time.Millisecond) @@ -177,7 +177,7 @@ func TestTransportSendAndFallback(t *testing.T) { if err != nil { t.Fatalf("newTransport: %v", err) } - if tr.send("sess", []byte("x")) { + if tr.send(frameFor(t, "sess", "x")) { t.Fatal("send must report false with no client connected") } diff --git a/internal/terminal/writequeue_test.go b/internal/terminal/writequeue_test.go index 0ac21bb46..fb73aa756 100644 --- a/internal/terminal/writequeue_test.go +++ b/internal/terminal/writequeue_test.go @@ -7,7 +7,7 @@ import ( "time" ) -// frameFor encodes one session frame the way send does. +// frameFor encodes one session frame the way an outbox does. func frameFor(t *testing.T, id, payload string) []byte { t.Helper() frame, err := encodeFrame(id, []byte(payload))