From c753fe4a4f589cc016a8346be222394902cb1f4d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 20:19:37 -0700 Subject: [PATCH 01/29] terminal: handle minimum row scroll delta PageList.scroll negated negative row deltas to obtain their magnitude. minInt(isize) has no positive signed representation, so callers could trigger a runtime safety panic before the existing traversal had a chance to clamp at the top. Use @abs to calculate an unsigned magnitude that represents every isize value. The same value now drives both cached-pin and general traversal paths. --- src/terminal/PageList.zig | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 04b58b99b..d722245b3 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -2907,6 +2907,8 @@ pub fn scroll(self: *PageList, behavior: Scroll) void { }, .delta_prompt => |n| self.scrollPrompt(n), .delta_row => |n| delta_row: { + const amount: usize = @abs(n); + switch (self.viewport) { // If we're at the top and we're scrolling backwards, // we don't have to do anything, because there's nowhere to go. @@ -2923,11 +2925,11 @@ pub fn scroll(self: *PageList, behavior: Scroll) void { .pin => switch (std.math.order(n, 0)) { .eq => break :delta_row, - .lt => switch (self.viewport_pin.upOverflow(@intCast(-n))) { + .lt => switch (self.viewport_pin.upOverflow(amount)) { .offset => |new_pin| { self.viewport_pin.* = new_pin; if (self.viewport_pin_row_offset) |*v| { - v.* -= @as(usize, @intCast(-n)); + v.* -= amount; } break :delta_row; }, @@ -2939,7 +2941,7 @@ pub fn scroll(self: *PageList, behavior: Scroll) void { }, }, - .gt => switch (self.viewport_pin.downOverflow(@intCast(n))) { + .gt => switch (self.viewport_pin.downOverflow(amount)) { // If we offset its a valid pin but we still have to // check if we're in the active area. .offset => |new_pin| { @@ -2948,7 +2950,7 @@ pub fn scroll(self: *PageList, behavior: Scroll) void { } else { self.viewport_pin.* = new_pin; if (self.viewport_pin_row_offset) |*v| { - v.* += @intCast(n); + v.* += amount; } } break :delta_row; @@ -2966,10 +2968,10 @@ pub fn scroll(self: *PageList, behavior: Scroll) void { // Slow path: we have to calculate the new pin by moving // from our viewport. const top = self.getTopLeft(.viewport); - const p: Pin = if (n < 0) switch (top.upOverflow(@intCast(-n))) { + const p: Pin = if (n < 0) switch (top.upOverflow(amount)) { .offset => |v| v, .overflow => |v| v.end, - } else switch (top.downOverflow(@intCast(n))) { + } else switch (top.downOverflow(amount)) { .offset => |v| v, .overflow => |v| v.end, }; @@ -8132,6 +8134,20 @@ test "PageList scroll delta row back overflow" { }, s.scrollbar()); } +test "PageList scroll minimum row delta" { + const testing = std.testing; + + var s = try init(testing.allocator, 10, 3, null); + defer s.deinit(); + + // Create one row of history so scrolling all the way back has an + // observable result. + try s.growRows(1); + s.scroll(.{ .delta_row = std.math.minInt(isize) }); + + try testing.expectEqual(Viewport.top, s.viewport); +} + test "PageList scroll delta row forward" { const testing = std.testing; const alloc = testing.allocator; From afbf5ba1564a87fd4c6d39117d6c66552b08ee71 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 20:20:46 -0700 Subject: [PATCH 02/29] terminal: handle minimum prompt scroll delta Prompt scrolling negated negative deltas to count the requested jumps. minInt(isize) has no positive signed representation, so a caller could trigger a runtime safety panic before the search for an earlier prompt started. Use @abs to produce the full unsigned magnitude. An extreme negative request now follows the normal prompt traversal and clamps at the oldest available prompt. --- src/terminal/PageList.zig | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index d722245b3..ff75f2c87 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -3009,7 +3009,7 @@ pub fn scroll(self: *PageList, behavior: Scroll) void { fn scrollPrompt(self: *PageList, delta: isize) void { // If we aren't jumping any prompts then we don't need to do anything. if (delta == 0) return; - const delta_start: usize = @intCast(if (delta > 0) delta else -delta); + const delta_start: usize = @abs(delta); var delta_rem: usize = delta_start; // We start at the row before or after our viewport depending on the @@ -8833,6 +8833,16 @@ test "PageList: jump zero prompts" { }, s.scrollbar()); } +test "PageList: jump minimum prompt delta" { + const testing = std.testing; + + var s = try init(testing.allocator, 10, 3, null); + defer s.deinit(); + + s.scroll(.{ .delta_prompt = std.math.minInt(isize) }); + try testing.expectEqual(Viewport.active, s.viewport); +} + test "Screen: jump back one prompt" { const testing = std.testing; const alloc = testing.allocator; From e6e4a9fdc1b5e793f54f3f981c70b640062b5354 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 20:23:06 -0700 Subject: [PATCH 03/29] terminal: widen cell screen coordinates Cell.screenPoint accumulated page row counts in CellCountInt even though screen point Y coordinates are u32. Once scrollback crossed 65,535 rows, walking back through page metadata overflowed and trapped in runtime safety builds. Accumulate directly in u32 so page-local u16 row counts widen before addition and the result uses the full range promised by point.Coordinate. --- src/terminal/PageList.zig | 43 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index ff75f2c87..0915f8023 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -6421,7 +6421,7 @@ pub const Cell = struct { /// this file then consider a different approach and ask yourself very /// carefully if you really need this. pub fn screenPoint(self: Cell) point.Point { - var y: size.CellCountInt = self.row_idx; + var y: u32 = self.row_idx; var node_ = self.node; while (node_.prev) |node| { y += node.rows(); @@ -9011,6 +9011,47 @@ test "PageList grow allocate" { } } +test "PageList Cell screenPoint supports long scrollback" { + const testing = std.testing; + + // A modest number of full-size page nodes is enough to exceed the u16 + // row range without allocating any page backing memory. screenPoint only + // reads the linked metadata while calculating the absolute coordinate. + const page_count = 307; + const rows_per_page = std_capacity.rows; + const nodes = try testing.allocator.alloc(Node, page_count); + defer testing.allocator.free(nodes); + + for (nodes, 0..) |*node, i| { + node.* = .{ + .prev = if (i > 0) &nodes[i - 1] else null, + .next = if (i + 1 < nodes.len) &nodes[i + 1] else null, + .data = .{ .resident = undefined }, + .serial = @intCast(i), + .owned = .heap, + }; + node.data.resident.size = .{ + .cols = 1, + .rows = rows_per_page, + }; + } + + const expected_y: u32 = (page_count - 1) * @as(u32, rows_per_page); + try testing.expect(expected_y > std.math.maxInt(size.CellCountInt)); + + const cell: Cell = .{ + .node = &nodes[nodes.len - 1], + .row = undefined, + .cell = undefined, + .row_idx = 0, + .col_idx = 0, + }; + try testing.expectEqual(point.Point{ .screen = .{ + .x = 0, + .y = expected_y, + } }, cell.screenPoint()); +} + test "PageList grow prune scrollback" { const testing = std.testing; const alloc = testing.allocator; From 30b42f42a3b33732154a07a556e6f768e6a0a949 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 20:24:48 -0700 Subject: [PATCH 04/29] terminal: reject unrepresentable pin coordinates pointFromPin accumulated scrollback rows directly into the u32 Y field. An unbounded PageList with more than 2^32 rows could overflow while converting a valid pin and panic in runtime safety builds. Use checked additions for every cross-page row contribution. If the pin cannot fit in point.Coordinate, return null through the existing out-of-range result instead of trapping. --- src/terminal/PageList.zig | 52 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 49 insertions(+), 3 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 0915f8023..d9066eb4c 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -4846,15 +4846,23 @@ pub fn pointFromPin(self: *const PageList, tag: point.Tag, p: Pin) ?point.Point if (tl.y > p.y) return null; coord.y = p.y - tl.y; } else { - coord.y += tl.node.rows() - tl.y; + coord.y = std.math.add( + u32, + coord.y, + tl.node.rows() - tl.y, + ) catch return null; var node_ = tl.node.next; while (node_) |node| : (node_ = node.next) { if (node == p.node) { - coord.y += p.y; + coord.y = std.math.add(u32, coord.y, p.y) catch return null; break; } - coord.y += node.rows(); + coord.y = std.math.add( + u32, + coord.y, + node.rows(), + ) catch return null; } else { // We never saw our node, meaning we're outside the range. return null; @@ -7804,6 +7812,44 @@ test "PageList pointFromPin traverse pages" { }) == null); } } + +test "PageList pointFromPin rejects overflowing screen coordinate" { + const testing = std.testing; + + // Use maximum-height metadata-only pages to model a valid scrollback just + // beyond the u32 coordinate range without allocating their backing cells. + const page_count = 65_539; + const rows_per_page = std.math.maxInt(size.CellCountInt); + const nodes = try testing.allocator.alloc(Node, page_count); + defer testing.allocator.free(nodes); + + for (nodes, 0..) |*node, i| { + node.* = .{ + .prev = if (i > 0) &nodes[i - 1] else null, + .next = if (i + 1 < nodes.len) &nodes[i + 1] else null, + .data = .{ .resident = undefined }, + .serial = @intCast(i), + .owned = .heap, + }; + node.data.resident.size = .{ + .cols = 1, + .rows = rows_per_page, + }; + } + + var s: PageList = undefined; + s.pages = .{ + .first = &nodes[0], + .last = &nodes[nodes.len - 1], + }; + + try testing.expect(s.pointFromPin(.screen, .{ + .node = &nodes[nodes.len - 1], + .y = 0, + .x = 0, + }) == null); +} + test "PageList active after grow" { const testing = std.testing; const alloc = testing.allocator; From d6e24d9856723055dbafc8d4133c440a265fa278 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 20:27:59 -0700 Subject: [PATCH 05/29] terminal: make pin traversal width-aware Pin movement assumed every page had the same column count. During an incomplete reflow, crossing into a narrower page could produce an out-of-bounds x coordinate, while wrapped movement could land on the wrong row or stop early. Use destination page widths while moving vertically or wrapping, and reject points that exceed the resolved page. Add synthetic mixed-width coverage for movement, wrapping, overflow, and point conversion. --- src/terminal/PageList.zig | 159 ++++++++++++++++++++++++++++---------- 1 file changed, 120 insertions(+), 39 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index d9066eb4c..f5203a809 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -4739,6 +4739,9 @@ pub fn pin(self: *const PageList, pt: point.Point) ?Pin { // Grab the top left and move to the point. var p = self.getTopLeft(pt).down(pt.coord().y) orelse return null; + // Incomplete reflow can leave a page narrower than the desired width. + // Never manufacture an out-of-bounds pin for that page. + if (x >= p.node.cols()) return null; p.x = x; return p; } @@ -6253,26 +6256,18 @@ pub const Pin = struct { /// /// TODO: Unit tests. pub fn leftWrap(self: Pin, n: usize) ?Pin { - // NOTE: This assumes that all pages have the same width, which may - // be violated under certain circumstances by incomplete reflow. - const cols = self.node.cols(); - const remaining_in_row = self.x; - - if (n <= remaining_in_row) return self.left(n); - - const extra_after_remaining = n - remaining_in_row; - const rows_off = 1 + (extra_after_remaining - 1) / cols; - - switch (self.upOverflow(rows_off)) { - .offset => |v| { - var result = v; - result.x = @intCast( - cols - 1 - (extra_after_remaining - 1) % cols, - ); - return result; - }, - .overflow => return null, + var result = self; + var remaining = n; + while (remaining > result.x) { + remaining -= @as(usize, result.x) + 1; + result = result.up(1) orelse return null; + // Crossing a row boundary lands on that destination row's final + // cell, whose width may differ from ours during reflow. + result.x = result.node.cols() - 1; } + + result.x -= @intCast(remaining); + return result; } /// Move the pin right n cells, wrapping to the next row as needed. @@ -6281,23 +6276,18 @@ pub const Pin = struct { /// /// TODO: Unit tests. pub fn rightWrap(self: Pin, n: usize) ?Pin { - // NOTE: This assumes that all pages have the same width, which may - // be violated under certain circumstances by incomplete reflow. - const cols = self.node.cols(); - const remaining_in_row = cols - self.x - 1; - - if (n <= remaining_in_row) return self.right(n); - - const extra_after_remaining = n - remaining_in_row; - const rows_off = 1 + (extra_after_remaining - 1) / cols; - - switch (self.downOverflow(rows_off)) { - .offset => |v| { - var result = v; - result.x = @intCast((extra_after_remaining - 1) % cols); + var result = self; + var remaining = n; + while (true) { + const row_remaining = result.node.cols() - result.x - 1; + if (remaining <= row_remaining) { + result.x += @intCast(remaining); return result; - }, - .overflow => return null, + } + + remaining -= @as(usize, row_remaining) + 1; + result = result.down(1) orelse return null; + result.x = 0; } } @@ -6345,7 +6335,7 @@ pub const Pin = struct { .end = .{ .node = node, .y = node.rows() - 1, - .x = self.x, + .x = @min(self.x, node.cols() - 1), }, .remaining = n_left, } }; @@ -6353,7 +6343,7 @@ pub const Pin = struct { .node = node, .y = std.math.cast(size.CellCountInt, n_left - 1) orelse std.math.maxInt(size.CellCountInt), - .x = self.x, + .x = @min(self.x, node.cols() - 1), } }; n_left -= node.rows(); } @@ -6381,20 +6371,111 @@ pub const Pin = struct { var n_left: usize = n - self.y; while (true) { node = node.prev orelse return .{ .overflow = .{ - .end = .{ .node = node, .y = 0, .x = self.x }, + .end = .{ + .node = node, + .y = 0, + .x = @min(self.x, node.cols() - 1), + }, .remaining = n_left, } }; if (n_left <= node.rows()) return .{ .offset = .{ .node = node, .y = std.math.cast(size.CellCountInt, node.rows() - n_left) orelse std.math.maxInt(size.CellCountInt), - .x = self.x, + .x = @min(self.x, node.cols() - 1), } }; n_left -= node.rows(); } } }; +fn mixedWidthPinListForTest(alloc: Allocator) !PageList { + var result = try init(alloc, 2, 1, null); + errdefer result.deinit(); + + // This deliberately constructs a layout that normal PageList operations + // do not expose yet. Keep integrity checks paused through deinit so the + // fixture can exercise mixed-width traversal in isolation. + result.pauseIntegrityChecks(true); + + inline for (.{ 4, 3 }) |cols| { + const node = try result.createPage(.{ .cap = .{ + .cols = cols, + .rows = 1, + } }); + node.page().size.rows = 1; + result.pages.append(node); + result.total_rows += 1; + } + + // Desired geometry is wider than the first and last stored pages. + result.cols = 4; + return result; +} + +test "PageList Pin row movement clamps across mixed-width pages" { + const testing = std.testing; + + var s = try mixedWidthPinListForTest(testing.allocator); + defer s.deinit(); + + const first = s.pages.first.?; + const second = first.next.?; + const third = second.next.?; + + try testing.expect((Pin{ .node = third, .x = 2 }).eql( + (Pin{ .node = second, .x = 3 }).down(1).?, + )); + try testing.expect((Pin{ .node = first, .x = 1 }).eql( + (Pin{ .node = second, .x = 3 }).up(1).?, + )); + + switch ((Pin{ .node = second, .x = 3 }).downOverflow(10)) { + .offset => try testing.expect(false), + .overflow => |overflow| try testing.expect( + (Pin{ .node = third, .x = 2 }).eql(overflow.end), + ), + } + switch ((Pin{ .node = second, .x = 3 }).upOverflow(10)) { + .offset => try testing.expect(false), + .overflow => |overflow| try testing.expect( + (Pin{ .node = first, .x = 1 }).eql(overflow.end), + ), + } +} + +test "PageList Pin wrapping crosses mixed-width pages" { + const testing = std.testing; + + var s = try mixedWidthPinListForTest(testing.allocator); + defer s.deinit(); + + const first = s.pages.first.?; + const second = first.next.?; + const third = second.next.?; + + try testing.expect((Pin{ .node = third, .x = 2 }).eql( + (Pin{ .node = first, .x = 1 }).rightWrap(7).?, + )); + try testing.expect((Pin{ .node = second, .x = 0 }).eql( + (Pin{ .node = third, .x = 2 }).leftWrap(6).?, + )); + try testing.expect((Pin{ .node = first }).leftWrap(1) == null); + try testing.expect((Pin{ .node = third, .x = 2 }).rightWrap(1) == null); +} + +test "PageList Pin rejects columns beyond mixed-width page bounds" { + const testing = std.testing; + + var s = try mixedWidthPinListForTest(testing.allocator); + defer s.deinit(); + + try testing.expect(s.pin(.{ .screen = .{ .x = 1, .y = 0 } }) != null); + try testing.expect(s.pin(.{ .screen = .{ .x = 2, .y = 0 } }) == null); + try testing.expect(s.pin(.{ .screen = .{ .x = 3, .y = 1 } }) != null); + try testing.expect(s.pin(.{ .screen = .{ .x = 3, .y = 2 } }) == null); +} + pub const Cell = struct { node: *List.Node, row: *pagepkg.Row, From 0cb004734eeb56f17bf1a72bbbd8cda6c924173e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 20:29:51 -0700 Subject: [PATCH 06/29] terminal: ignore empty cell clears Screen.clearCells accepted a slice but its runtime safety validation indexed the first and last elements unconditionally. Passing an empty range therefore panicked before the otherwise valid no-op clear. Return immediately for an empty slice so validation and managed-memory bookkeeping only run when there are cells to clear. --- src/terminal/Screen.zig | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 95cba1bf9..8880e348c 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -1546,6 +1546,8 @@ pub fn clearCells( row: *Row, cells: []Cell, ) void { + if (cells.len == 0) return; + // This whole operation does unsafe things, so we just want to assert // the end state. page.pauseIntegrityChecks(true); @@ -4084,6 +4086,18 @@ test "Screen clearRows active styled line" { try testing.expectEqualStrings("", str); } +test "Screen clearCells empty range" { + const testing = std.testing; + + var s = try Screen.init(testing.allocator, .default); + defer s.deinit(); + + const page = s.cursor.page_pin.node.page(); + const row = s.cursor.page_row; + const cells = page.getCells(row); + s.clearCells(page, row, cells[0..0]); +} + test "Screen clearRows protected" { const testing = std.testing; const alloc = testing.allocator; From 8f1c2fe959f28d3d12c1a183d7213eb36c74402e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 20:48:58 -0700 Subject: [PATCH 07/29] terminal: handle backwards selection timestamps SelectionGesture passed caller-supplied repeat timestamps directly to Instant.since. A C API client or non-monotonic timer could provide an earlier timestamp after a later one, causing a runtime safety panic while converting negative elapsed seconds to u64. Compare instants before calculating elapsed time and treat backwards timestamps as failed repeats. The next press becomes a new single-click anchor, matching other invalid repeat inputs. --- src/terminal/SelectionGesture.zig | 41 ++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/src/terminal/SelectionGesture.zig b/src/terminal/SelectionGesture.zig index f0e11f63e..3fe4b7298 100644 --- a/src/terminal/SelectionGesture.zig +++ b/src/terminal/SelectionGesture.zig @@ -228,7 +228,8 @@ pub fn validatedLeftClickPin( } pub const Press = struct { - /// The time when the press event occurred. Use a monotonic timer. + /// The time when the press event occurred. Prefer a monotonic timer; + /// backwards timestamps reset the repeat sequence. /// This can be null if you're on a system that doesn't support /// time for some reason. In that case, we only support single clicks. time: ?Time, @@ -630,7 +631,8 @@ fn pressRepeat( const time = p.time orelse return error.PressRequiresReset; { const prev_time = self.left_click_time orelse return error.PressRequiresReset; - const since = timeSince(time, prev_time); + const since = timeSince(time, prev_time) orelse + return error.PressRequiresReset; if (since > p.repeat_interval) return error.PressRequiresReset; } @@ -664,8 +666,13 @@ fn pressRepeat( self.left_click_behavior = p.behaviors[self.left_click_count - 1]; } -fn timeSince(time: Time, prev_time: Time) u64 { - if (comptime freestanding_wasm) return time -| prev_time; +fn timeSince(time: Time, prev_time: Time) ?u64 { + if (comptime freestanding_wasm) { + if (time < prev_time) return null; + return time - prev_time; + } + + if (time.order(prev_time) == .lt) return null; return time.since(prev_time); } @@ -935,6 +942,16 @@ fn testPress(t: *Terminal, x: u16, y: u32, time: ?std.time.Instant) Press { }; } +fn testInstant(ns: u64) std.time.Instant { + return switch (builtin.os.tag) { + .windows, .uefi, .wasi => .{ .timestamp = ns }, + else => .{ .timestamp = .{ + .sec = @intCast(ns / std.time.ns_per_s), + .nsec = @intCast(ns % std.time.ns_per_s), + } }, + }; +} + fn testDrag(t: *Terminal, x: u16, y: u32, xpos: f64, ypos: f64) Drag { return .{ .pin = t.screens.active.pages.pin(.{ .active = .{ @@ -1965,6 +1982,22 @@ test "SelectionGesture expired repeat resets click count" { try testing.expectEqual(@as(u3, 1), gesture.left_click_count); } +test "SelectionGesture backwards repeat time resets click count" { + var t = try Terminal.init(testing.allocator, .{ .cols = 5, .rows = 5 }); + defer t.deinit(testing.allocator); + + var gesture: SelectionGesture = .init; + defer gesture.deinit(&t); + + const earlier = testInstant(std.time.ns_per_s); + const later = testInstant(2 * std.time.ns_per_s); + _ = try gesture.press(&t, testPress(&t, 1, 1, later)); + _ = try gesture.press(&t, testPress(&t, 1, 1, earlier)); + + try testing.expectEqual(@as(u3, 1), gesture.left_click_count); + try testing.expectEqual(.eq, gesture.left_click_time.?.order(earlier)); +} + test "SelectionGesture screen switch resets click count" { var t = try Terminal.init(testing.allocator, .{ .cols = 5, .rows = 5 }); defer t.deinit(testing.allocator); From 3d08161b5053ea8930b97232ab88486d2ff6a7bf Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 20:50:23 -0700 Subject: [PATCH 08/29] terminal: handle empty tabstop ranges Tabstops.reset subtracted one from the column count before iterating default stops. Although init and resize accept zero columns, resetting that state with a nonzero interval underflowed and panicked. Return after clearing when the grid has fewer than two columns. Empty and single-column tabstop sets now preserve the normal no-stop result. --- src/terminal/Tabstops.zig | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/terminal/Tabstops.zig b/src/terminal/Tabstops.zig index 68138cbf8..8dbe67893 100644 --- a/src/terminal/Tabstops.zig +++ b/src/terminal/Tabstops.zig @@ -170,11 +170,11 @@ pub fn reset(self: *Tabstops, interval: usize) void { @memset(&self.prealloc_stops, 0); @memset(self.dynamic_stops, 0); - if (interval > 0) { - var i: usize = interval; - while (i < self.cols - 1) : (i += interval) { - self.set(i); - } + if (interval == 0 or self.cols <= 1) return; + + var i: usize = interval; + while (i < self.cols - 1) : (i += interval) { + self.set(i); } } @@ -230,6 +230,13 @@ test "Tabstops: interval" { try testing.expect(t.get(8)); } +test "Tabstops: interval with zero columns" { + var t: Tabstops = try init(testing.allocator, 0, 8); + defer t.deinit(testing.allocator); + + try testing.expectEqual(@as(usize, 0), t.cols); +} + test "Tabstops: count on 80" { // https://superuser.com/questions/710019/why-there-are-11-tabstops-on-a-80-column-console From fa8cae88b25f7c8fba3328f9e27edcb66a148341 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 20:54:09 -0700 Subject: [PATCH 09/29] terminal: use destination width for line selection Semantic line selection moved to the previous row when the next row started with different content, then assigned the previous pin an x coordinate from the next page. During incomplete reflow, a wider next page produced an out-of-bounds pin and a runtime safety panic. Set the end column from the page that owns the destination pin. Line selection now remains valid while crossing mixed-width page boundaries. --- src/terminal/Screen.zig | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 8880e348c..eb5e4cf52 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -2809,7 +2809,7 @@ pub fn selectLine(self: *const Screen, opts: SelectLine) ?Selection { // so we scan forward to find where our content ends. if (start_offset == 0 and cells[0].semantic_content != v) { var prev = p.up(1).?; - prev.x = p.node.cols() - 1; + prev.x = prev.node.cols() - 1; break :end_pin prev; } @@ -8818,6 +8818,37 @@ test "Screen: selectLine semantic boundary first cell of row" { } } +test "Screen: selectLine semantic boundary across mixed-width pages" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, .{ .cols = 4, .rows = 2, .max_scrollback = 0 }); + defer s.deinit(); + + s.cursorSetSemanticContent(.{ .input = .clear_explicit }); + try s.testWriteString("ABCD"); + s.cursorSetSemanticContent(.output); + try s.testWriteString("E"); + + const first = s.pages.pages.first.?; + try s.pages.split(.{ .node = first, .y = 1 }); + const second = first.next.?; + first.page().size.cols = 2; + s.pages.pauseIntegrityChecks(true); + defer s.pages.pauseIntegrityChecks(false); + + try testing.expectEqual(@as(size.CellCountInt, 2), first.cols()); + try testing.expectEqual(@as(size.CellCountInt, 4), second.cols()); + + var sel = s.selectLine(.{ .pin = .{ + .node = first, + .x = 1, + } }).?; + defer sel.deinit(&s); + try testing.expect((Pin{ .node = first, .x = 0 }).eql(sel.start())); + try testing.expect((Pin{ .node = first, .x = 1 }).eql(sel.end())); +} + test "Screen: selectLine semantic all same content" { const testing = std.testing; const alloc = testing.allocator; From a9f5b7eba26ed50b73158f0f8a564c00106db7a0 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 20:56:44 -0700 Subject: [PATCH 10/29] terminal: clamp selection rows to page width containedRowCached built full-row and rectangular selections using the desired screen width. During incomplete reflow, an intermediate narrower page received endpoints beyond its cell range, and consumers could panic when resolving the returned pins. Use the owning page width for full rows and clamp rectangular bounds to that width. Every contained-row selection now returns resolvable pins. --- src/terminal/Selection.zig | 52 +++++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 4 deletions(-) diff --git a/src/terminal/Selection.zig b/src/terminal/Selection.zig index 8aacc9ba9..17f6e462f 100644 --- a/src/terminal/Selection.zig +++ b/src/terminal/Selection.zig @@ -326,6 +326,7 @@ pub fn containedRowCached( br: point.Coordinate, p: point.Coordinate, ) ?Selection { + _ = s; if (p.y < tl.y or p.y > br.y) return null; // Rectangle case: we can return early as the x range will always be the @@ -333,12 +334,12 @@ pub fn containedRowCached( if (self.rectangle) return init( start: { var copy: Pin = pin; - copy.x = tl.x; + copy.x = @min(tl.x, copy.node.cols() - 1); break :start copy; }, end: { var copy: Pin = pin; - copy.x = br.x; + copy.x = @min(br.x, copy.node.cols() - 1); break :end copy; }, true, @@ -355,7 +356,7 @@ pub fn containedRowCached( tl_pin, end: { var copy: Pin = pin; - copy.x = s.pages.cols - 1; + copy.x = copy.node.cols() - 1; break :end copy; }, false, @@ -387,7 +388,7 @@ pub fn containedRowCached( }, end: { var copy: Pin = pin; - copy.x = s.pages.cols - 1; + copy.x = copy.node.cols() - 1; break :end copy; }, false, @@ -1602,3 +1603,46 @@ test "Selection: containedRow" { ).?); } } + +test "Selection: containedRow clamps mixed-width pages" { + const testing = std.testing; + var s = try Screen.init(testing.allocator, .{ + .cols = 4, + .rows = 3, + .max_scrollback = 0, + }); + defer s.deinit(); + + const first = s.pages.pages.first.?; + try s.pages.split(.{ .node = first, .y = 2 }); + try s.pages.split(.{ .node = first, .y = 1 }); + const middle = first.next.?; + const last = middle.next.?; + middle.page().size.cols = 2; + + const linear = Selection.init( + .{ .node = first, .x = 1 }, + .{ .node = last, .x = 1 }, + false, + ); + const linear_row = linear.containedRow( + &s, + .{ .node = middle }, + ).?; + _ = linear_row.end().rowAndCell(); + try testing.expect((Pin{ .node = middle }).eql(linear_row.start())); + try testing.expect((Pin{ .node = middle, .x = 1 }).eql(linear_row.end())); + + const rectangle = Selection.init( + .{ .node = first, .x = 1 }, + .{ .node = last, .x = 3 }, + true, + ); + const rectangle_row = rectangle.containedRow( + &s, + .{ .node = middle }, + ).?; + _ = rectangle_row.end().rowAndCell(); + try testing.expect((Pin{ .node = middle, .x = 1 }).eql(rectangle_row.start())); + try testing.expect((Pin{ .node = middle, .x = 1 }).eql(rectangle_row.end())); +} From a55850c981f2ee39b3c47c7fe0b0fef13570f311 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 20:58:14 -0700 Subject: [PATCH 11/29] terminal: use previous page width for cursor cells cursorCellEndOfPrev moved its pin to the previous row but then set the column from the desired screen width. If incomplete reflow left that previous page narrower, resolving the cell used an out-of-bounds column and panicked in runtime safety builds. Set the column from the page reached by the cursor pin so the returned cell is always the actual final cell of that row. --- src/terminal/Screen.zig | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index eb5e4cf52..ec4ad24d9 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -651,7 +651,7 @@ pub fn cursorCellEndOfPrev(self: *Screen) *pagepkg.Cell { assert(self.cursor.y > 0); var page_pin = self.cursor.page_pin.up(1).?; - page_pin.x = self.pages.cols - 1; + page_pin.x = page_pin.node.cols() - 1; const page_rac = page_pin.rowAndCell(); return page_rac.cell; } @@ -4223,6 +4223,27 @@ test "Screen eraseRows active partial" { } } +test "Screen: cursorCellEndOfPrev across mixed-width pages" { + const testing = std.testing; + var s = try init(testing.allocator, .{ + .cols = 4, + .rows = 2, + .max_scrollback = 0, + }); + defer s.deinit(); + + try s.testWriteString("ABCDE"); + const first = s.pages.pages.first.?; + try s.pages.split(.{ .node = first, .y = 1 }); + s.cursorReload(); + const second = first.next.?; + first.page().size.cols = 2; + + try testing.expectEqual(second, s.cursor.page_pin.node); + const expected = (Pin{ .node = first, .x = 1 }).rowAndCell().cell; + try testing.expectEqual(expected, s.cursorCellEndOfPrev()); +} + test "Screen: cursorDown across pages preserves style" { const testing = std.testing; const alloc = testing.allocator; From 08e376f2398d210cb1f67d3d14c901d3c70f071e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 21:01:02 -0700 Subject: [PATCH 12/29] terminal: bound selection drag geometry Selection drags converted caller-provided floating-point positions directly to u32 and multiplied geometry dimensions without checking their range. Non-finite positions, oversized values, overflowing dimensions, or empty core geometry could therefore panic in runtime safety builds. Clamp pixel positions to the representable u32 range, reject empty geometry, and saturate the pixel span when dimensions overflow. Valid drags keep their existing threshold behavior. --- src/terminal/SelectionGesture.zig | 78 +++++++++++++++++++++++++++++-- 1 file changed, 75 insertions(+), 3 deletions(-) diff --git a/src/terminal/SelectionGesture.zig b/src/terminal/SelectionGesture.zig index 3fe4b7298..2be1e8aa3 100644 --- a/src/terminal/SelectionGesture.zig +++ b/src/terminal/SelectionGesture.zig @@ -388,8 +388,8 @@ pub fn drag( .cell => dragSelection( click_pin.*, d.pin, - @intFromFloat(@max(0, self.left_click_xpos)), - @intFromFloat(@max(0, d.xpos)), + pixelFromFloat(self.left_click_xpos), + pixelFromFloat(d.xpos), d.rectangle, d.geometry, ), @@ -676,6 +676,19 @@ fn timeSince(time: Time, prev_time: Time) ?u64 { return time.since(prev_time); } +/// Convert a caller-provided floating-point position to a pixel coordinate. +/// Negative and NaN values clamp to the origin, matching the drag behavior +/// outside the left edge of the surface. +fn pixelFromFloat(value: f64) u32 { + if (std.math.isNan(value) or value <= 0) return 0; + + // @intFromFloat requires a value representable by the destination type. + // Saturate first so positive infinity and oversized coordinates are safe. + const max: f64 = @floatFromInt(std.math.maxInt(u32)); + if (value >= max) return std.math.maxInt(u32); + return @intFromFloat(value); +} + fn pressSelection( self: *const SelectionGesture, screen: *Screen, @@ -720,6 +733,8 @@ fn dragSelection( // Rectangular selections are handled similarly, except that // entire columns are considered rather than individual cells. + if (geometry.columns == 0 or geometry.cell_width == 0) return null; + // We only include cells in the selection if the threshold point lies // between the start and end points of the selection. A threshold of // 60% of the cell width was chosen empirically because it felt good. @@ -728,7 +743,12 @@ fn dragSelection( )); // We use this to clamp the pixel positions below. - const max_x = geometry.columns * geometry.cell_width - 1; + const pixel_span = std.math.mul( + u32, + geometry.columns, + geometry.cell_width, + ) catch std.math.maxInt(u32); + const max_x = pixel_span - 1; // We need to know how far across in the cell the drag pos is, so // we subtract the padding and then take it modulo the cell width. @@ -1549,6 +1569,58 @@ test "SelectionGesture drag returns selection and records autoscroll" { try testing.expectEqual(.down, gesture.left_drag_autoscroll); } +test "SelectionGesture drag clamps unrepresentable positions" { + var t = try Terminal.init(testing.allocator, .{ .cols = 5, .rows = 5 }); + defer t.deinit(testing.allocator); + + var gesture: SelectionGesture = .init; + defer gesture.deinit(&t); + + var press_event = testPress(&t, 1, 1, try std.time.Instant.now()); + press_event.xpos = 10; + _ = try gesture.press(&t, press_event); + + const positive = gesture.drag( + &t, + testDrag(&t, 1, 1, std.math.inf(f64), 50), + ).?; + try testing.expect((testPin(&t, 1, 1)).eql(positive.start())); + try testing.expect((testPin(&t, 1, 1)).eql(positive.end())); + + try testing.expectEqual(null, gesture.drag( + &t, + testDrag(&t, 1, 1, std.math.nan(f64), 50), + )); +} + +test "SelectionGesture drag saturates overflowing geometry" { + var t = try Terminal.init(testing.allocator, .{ .cols = 5, .rows = 5 }); + defer t.deinit(testing.allocator); + + var gesture: SelectionGesture = .init; + defer gesture.deinit(&t); + + _ = try gesture.press(&t, testPress(&t, 1, 1, try std.time.Instant.now())); + var drag_event = testDrag(&t, 1, 1, 10, 50); + drag_event.geometry.columns = std.math.maxInt(u32); + drag_event.geometry.cell_width = std.math.maxInt(u32); + try testing.expectEqual(null, gesture.drag(&t, drag_event)); +} + +test "SelectionGesture drag rejects empty geometry" { + var t = try Terminal.init(testing.allocator, .{ .cols = 5, .rows = 5 }); + defer t.deinit(testing.allocator); + + var gesture: SelectionGesture = .init; + defer gesture.deinit(&t); + + _ = try gesture.press(&t, testPress(&t, 1, 1, try std.time.Instant.now())); + var drag_event = testDrag(&t, 1, 1, 10, 50); + drag_event.geometry.columns = 0; + drag_event.geometry.cell_width = 0; + try testing.expectEqual(null, gesture.drag(&t, drag_event)); +} + test "SelectionGesture release clears autoscroll and records drag" { var t = try Terminal.init(testing.allocator, .{ .cols = 5, .rows = 5 }); defer t.deinit(testing.allocator); From 6071606577d80d75865bdd91bd5f7b6df4a0d60e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 21:03:28 -0700 Subject: [PATCH 13/29] terminal: clamp cloned selections to page width Screen.clone clipped selections with the desired screen width or copied rectangle columns unchanged. PageList.clone preserves stored page widths, so a clipped boundary on a narrower page produced an invalid tracked pin and panicked during runtime validation. Build fallback pins from the first or last cloned node and clamp rectangle columns to that node. Clipped selections now remain valid during reflow. --- src/terminal/Screen.zig | 69 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 62 insertions(+), 7 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index ec4ad24d9..da1563608 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -530,20 +530,30 @@ pub fn clone( } // We move the top pin back in bounds to the top row. + const node = pages.pages.first.?; break :start try pages.trackPin(.{ - .node = pages.pages.first.?, - .x = if (sel.rectangle) ordered.tl.x else 0, + .node = node, + .x = if (sel.rectangle) + @min(ordered.tl.x, node.cols() - 1) + else + 0, }); }; // If we got to this point it means that the selection is not // fully out of bounds, so we move the bottom right pin back // in bounds if it isn't already. - const end_pin = pin_remap.get(ordered.br) orelse try pages.trackPin(.{ - .node = pages.pages.last.?, - .x = if (sel.rectangle) ordered.br.x else pages.cols - 1, - .y = pages.pages.last.?.rows() - 1, - }); + const end_pin = pin_remap.get(ordered.br) orelse end: { + const node = pages.pages.last.?; + break :end try pages.trackPin(.{ + .node = node, + .x = if (sel.rectangle) + @min(ordered.br.x, node.cols() - 1) + else + node.cols() - 1, + .y = node.rows() - 1, + }); + }; break :sel .{ .bounds = .{ .tracked = .{ @@ -5969,6 +5979,51 @@ test "Screen: clone contains subset of selection" { } } +test "Screen: clone clamps clipped selections to mixed-width pages" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, .{ .cols = 4, .rows = 3, .max_scrollback = 0 }); + defer s.deinit(); + + const first = s.pages.pages.first.?; + try s.pages.split(.{ .node = first, .y = 2 }); + try s.pages.split(.{ .node = first, .y = 1 }); + const middle = first.next.?; + const last = middle.next.?; + middle.page().size.cols = 2; + + try s.select(Selection.init( + .{ .node = first }, + .{ .node = last, .x = 3 }, + false, + )); + var linear = try s.clone( + alloc, + .{ .screen = .{} }, + .{ .screen = .{ .y = 1 } }, + ); + defer linear.deinit(); + const linear_end = linear.selection.?.end(); + _ = linear_end.rowAndCell(); + try testing.expectEqual(@as(size.CellCountInt, 1), linear_end.x); + + try s.select(Selection.init( + .{ .node = first, .x = 3 }, + .{ .node = last, .x = 3 }, + true, + )); + var rectangle = try s.clone( + alloc, + .{ .screen = .{ .y = 1 } }, + null, + ); + defer rectangle.deinit(); + const rectangle_start = rectangle.selection.?.start(); + _ = rectangle_start.rowAndCell(); + try testing.expectEqual(@as(size.CellCountInt, 1), rectangle_start.x); +} + test "Screen: clone contains subset of rectangle selection" { const testing = std.testing; const alloc = testing.allocator; From b6f34be44f8efc16e58072d0681d4bb0ab076f75 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 21:05:51 -0700 Subject: [PATCH 14/29] terminal: clamp mirrored selection corners Rectangle orientation swaps endpoint columns when a selection is mirrored. During incomplete reflow, copying a column from a wider page onto the narrower corner page created an invalid pin that panicked when resolved. Clamp every swapped column to the page that owns the oriented corner. Top-left and bottom-right calculations now always return valid pins. --- src/terminal/Selection.zig | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/src/terminal/Selection.zig b/src/terminal/Selection.zig index 17f6e462f..be6854e5f 100644 --- a/src/terminal/Selection.zig +++ b/src/terminal/Selection.zig @@ -155,12 +155,12 @@ pub fn topLeft(self: Selection, s: *const Screen) Pin { .reverse => self.end(), .mirrored_forward => pin: { var p = self.start(); - p.x = self.end().x; + p.x = @min(self.end().x, p.node.cols() - 1); break :pin p; }, .mirrored_reverse => pin: { var p = self.end(); - p.x = self.start().x; + p.x = @min(self.start().x, p.node.cols() - 1); break :pin p; }, }; @@ -173,12 +173,12 @@ pub fn bottomRight(self: Selection, s: *const Screen) Pin { .reverse => self.start(), .mirrored_forward => pin: { var p = self.end(); - p.x = self.start().x; + p.x = @min(self.start().x, p.node.cols() - 1); break :pin p; }, .mirrored_reverse => pin: { var p = self.start(); - p.x = self.end().x; + p.x = @min(self.end().x, p.node.cols() - 1); break :pin p; }, }; @@ -1055,6 +1055,32 @@ test "Selection: order, standard" { } } +test "Selection: rectangle corners clamp across mixed-width pages" { + const testing = std.testing; + var s = try Screen.init(testing.allocator, .{ + .cols = 4, + .rows = 2, + .max_scrollback = 0, + }); + defer s.deinit(); + + const first = s.pages.pages.first.?; + try s.pages.split(.{ .node = first, .y = 1 }); + const second = first.next.?; + second.page().size.cols = 2; + + const sel = Selection.init( + .{ .node = first, .x = 3 }, + .{ .node = second, .x = 1 }, + true, + ); + try testing.expectEqual(.mirrored_forward, sel.order(&s)); + + const bottom_right = sel.bottomRight(&s); + _ = bottom_right.rowAndCell(); + try testing.expect((Pin{ .node = second, .x = 1 }).eql(bottom_right)); +} + test "Selection: order, rectangle" { const testing = std.testing; const alloc = testing.allocator; From e727a36589dffa5b42f76e83d260a5617cdde54e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 21:07:43 -0700 Subject: [PATCH 15/29] terminal: clear rows using stored page width Screen.clearRows sliced backing cells using the desired PageList width. During incomplete reflow, clearing a narrower stored page extended the slice past its row and tripped clearCells runtime validation before any cells were cleared. Obtain cells from the owning page and use its stored width for whole-row managed-memory bookkeeping. Mixed-width history rows now clear safely. --- src/terminal/Screen.zig | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index da1563608..d8ce4db9a 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -1526,19 +1526,19 @@ pub fn clearRows( var it = self.pages.pageIterator(.right_down, tl, bl); while (it.next()) |chunk| { + const page = chunk.node.page(); for (chunk.rows()) |*row| { const cells_offset = row.cells; - const cells_multi: [*]Cell = row.cells.ptr(chunk.node.page().memory); - const cells = cells_multi[0..self.pages.cols]; + const cells = page.getCells(row); // Clear all cells if (protected) { - self.clearUnprotectedCells(chunk.node.page(), row, cells); + self.clearUnprotectedCells(page, row, cells); // We need to preserve other row attributes since we only // cleared unprotected cells. row.cells = cells_offset; } else { - self.clearCells(chunk.node.page(), row, cells); + self.clearCells(page, row, cells); row.* = .{ .cells = cells_offset }; } @@ -1589,7 +1589,7 @@ pub fn clearCells( // If we have no left/right scroll region we can be sure // that we've cleared all the graphemes, so we clear the // flag, otherwise we ask the page to update the flag. - if (cells.len == self.pages.cols) { + if (cells.len == page.size.cols) { row.grapheme = false; } else { page.updateRowGraphemeFlag(row); @@ -1605,7 +1605,7 @@ pub fn clearCells( // If we have no left/right scroll region we can be sure // that we've cleared all the hyperlinks, so we clear the // flag, otherwise we ask the page to update the flag. - if (cells.len == self.pages.cols) { + if (cells.len == page.size.cols) { row.hyperlink = false; } else { page.updateRowHyperlinkFlag(row); @@ -1633,7 +1633,7 @@ pub fn clearCells( // If we have no left/right scroll region we can be sure // that we've cleared all the styles, so we clear the // flag, otherwise we ask the page to update the flag. - if (cells.len == self.pages.cols) { + if (cells.len == page.size.cols) { row.styled = false; } else { page.updateRowStyledFlag(row); @@ -1642,7 +1642,7 @@ pub fn clearCells( if (comptime build_options.kitty_graphics) { if (row.kitty_virtual_placeholder and - cells.len == self.pages.cols) + cells.len == page.size.cols) { for (cells) |c| { if (c.codepoint() == kitty.graphics.unicode.placeholder) { @@ -4108,6 +4108,26 @@ test "Screen clearCells empty range" { s.clearCells(page, row, cells[0..0]); } +test "Screen clearRows uses stored page width" { + const testing = std.testing; + + var s = try Screen.init(testing.allocator, .{ + .cols = 2, + .rows = 1, + .max_scrollback = 0, + }); + defer s.deinit(); + + try s.testWriteString("AB"); + const node = s.pages.pages.first.?; + s.pages.cols = 4; + + s.clearRows(.{ .screen = .{} }, null, false); + const cells = node.page().getCells(&node.page().rows.ptr(node.page().memory)[0]); + try testing.expectEqual(@as(usize, 2), cells.len); + for (cells) |cell| try testing.expect(cell.isEmpty()); +} + test "Screen clearRows protected" { const testing = std.testing; const alloc = testing.allocator; From b287f6d1ab98e92b05ae6a2b4ea29d4daad9d3bd Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 21:34:39 -0700 Subject: [PATCH 16/29] terminal: handle stored grapheme boundaries With mode 2027 disabled, the printer attaches zero-width codepoints without applying Unicode grapheme boundaries. Enabling the mode later and printing another non-ASCII codepoint asserted that every stored pair was part of one grapheme and panicked when it was not. Feed all stored codepoints through the grapheme state machine and let it reset at existing boundaries before testing the new codepoint. The deterministic print comparison can now exercise live mode changes without clearing the screen first. --- src/terminal/Terminal.zig | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 3bcfdf5d9..05085721a 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -952,7 +952,13 @@ pub fn print(self: *Terminal, c: u21) !void { const cps = self.screens.active.cursor.page_pin.node.page().lookupGrapheme(prev.cell).?; for (cps) |cp2| { // log.debug("cp1={x} cp2={x}", .{ previous_codepoint, cp2 }); - assert(!unicode.graphemeBreak(previous_codepoint, cp2, &state)); + // With mode 2027 disabled, zero-width codepoints are + // attached without applying grapheme boundary rules. If + // the mode is enabled later, an existing cell can + // therefore contain one or more breaks. Feed those breaks + // into the state machine so it can reset its context and + // determine the boundary for the new codepoint. + _ = unicode.graphemeBreak(previous_codepoint, cp2, &state); previous_codepoint = cp2; } } @@ -4297,6 +4303,22 @@ test "Terminal: print multicodepoint grapheme, disabled mode 2027" { try testing.expect(t.isDirty(.{ .screen = .{ .x = 0, .y = 0 } })); } +test "Terminal: enabling grapheme mode handles stored breaks" { + var t = try init(testing.allocator, .{ .cols = 5, .rows = 1 }); + defer t.deinit(testing.allocator); + + t.modes.set(.grapheme_cluster, false); + try t.print('a'); + try t.print(0x200B); // Zero width space is stored on the prior cell. + + t.modes.set(.grapheme_cluster, true); + try t.print(0x0301); + + const str = try t.plainString(testing.allocator); + defer testing.allocator.free(str); + try testing.expectEqualStrings("a\xE2\x80\x8B\xCC\x81", str); +} + // Terminal.print receives one codepoint at a time, so it can't use // unicode.graphemeWidth directly; that API requires a complete buffered // cluster or string end. This keeps the streaming printer's cursor advance @@ -12491,13 +12513,6 @@ fn testPrintSliceDifferential( }, 15 => { const v = rand.boolean(); - // Erase the display first: grapheme clusters created - // while mode 2027 was off can trip a pre-existing - // debug assert in print()'s cluster walk when the mode - // is toggled on (unrelated to printSlice; it reproduces - // with per-codepoint print alone). - t1.eraseDisplay(.complete, false); - t2.eraseDisplay(.complete, false); t1.modes.set(.grapheme_cluster, v); t2.modes.set(.grapheme_cluster, v); }, From 04810273afc2c1dba73cf32ebb6d63ceb37df168 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 21:37:26 -0700 Subject: [PATCH 17/29] terminal: wrap implicit hyperlink identifiers Implicit OSC 8 hyperlinks incremented a u32 identifier with checked arithmetic even though the cursor contract allows the sequence to wrap. Reaching the maximum identifier therefore caused a runtime safety panic before the link could be installed. Use wrapping arithmetic for both the successful increment and the error rollback. The identifier now returns to zero at the boundary, while failed allocation attempts still restore the original value. --- src/terminal/Screen.zig | 41 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index d8ce4db9a..b47e4e1cd 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -2413,13 +2413,13 @@ pub fn startHyperlink( .id = if (id_) |id| .{ .explicit = id, } else implicit: { - defer self.cursor.hyperlink_implicit_id += 1; + defer self.cursor.hyperlink_implicit_id +%= 1; break :implicit .{ .implicit = self.cursor.hyperlink_implicit_id }; }, }; errdefer switch (link.id) { .explicit => {}, - .implicit => self.cursor.hyperlink_implicit_id -= 1, + .implicit => self.cursor.hyperlink_implicit_id -%= 1, }; // Loop until we have enough page memory to add the hyperlink @@ -9996,6 +9996,43 @@ test "Screen: hyperlink start/end" { } } +test "Screen: implicit hyperlink ID wraps" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, .{ .cols = 5, .rows = 5, .max_scrollback = 0 }); + defer s.deinit(); + + s.cursor.hyperlink_implicit_id = std.math.maxInt(size.OffsetInt); + try s.startHyperlink("http://example.com", null); + + try testing.expectEqual(@as(size.OffsetInt, 0), s.cursor.hyperlink_implicit_id); + try testing.expectEqual( + std.math.maxInt(size.OffsetInt), + s.cursor.hyperlink.?.id.implicit, + ); + + // A failed allocation must roll the wrapped counter back to its + // original value as well. + s.endHyperlink(); + s.cursor.hyperlink_implicit_id = std.math.maxInt(size.OffsetInt); + var failing = testing.FailingAllocator.init(alloc, .{}); + failing.fail_index = failing.alloc_index; + { + const original_alloc = s.alloc; + defer s.alloc = original_alloc; + s.alloc = failing.allocator(); + try testing.expectError( + error.OutOfMemory, + s.startHyperlink("http://example.com", null), + ); + } + try testing.expectEqual( + std.math.maxInt(size.OffsetInt), + s.cursor.hyperlink_implicit_id, + ); +} + test "Screen: hyperlink reuse" { const testing = std.testing; const alloc = testing.allocator; From 1d5b6f90e5278fc53820605fae47ca0fa5b926a7 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 21:39:34 -0700 Subject: [PATCH 18/29] terminal: reserve pwd terminator atomically setPwd appended the path bytes and terminating NUL with separate fallible operations. If only the terminator allocation failed, the function returned OutOfMemory but left a nonempty unterminated buffer that made getPwd panic on its sentinel check. Reserve checked capacity for the complete sentinel-terminated value before clearing the old state, then use infallible appends. Allocation failure now leaves a valid prior value instead of exposing partial data. --- src/terminal/Terminal.zig | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 05085721a..b79f21bb7 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -3556,11 +3556,18 @@ pub fn resize( /// Set the pwd for the terminal. pub fn setPwd(self: *Terminal, pwd: []const u8) !void { - self.pwd.clearRetainingCapacity(); - if (pwd.len > 0) { - try self.pwd.appendSlice(self.gpa(), pwd); - try self.pwd.append(self.gpa(), 0); + if (pwd.len == 0) { + self.pwd.clearRetainingCapacity(); + return; } + + const capacity = std.math.add(usize, pwd.len, 1) catch + return error.OutOfMemory; + try self.pwd.ensureTotalCapacity(self.gpa(), capacity); + + self.pwd.clearRetainingCapacity(); + self.pwd.appendSliceAssumeCapacity(pwd); + self.pwd.appendAssumeCapacity(0); } /// Returns the pwd for the terminal, if any. The memory is owned by the @@ -3570,6 +3577,18 @@ pub fn getPwd(self: *const Terminal) ?[:0]const u8 { return self.pwd.items[0 .. self.pwd.items.len - 1 :0]; } +test "Terminal: setPwd preserves a sentinel on allocation failure" { + var failing = testing.FailingAllocator.init(testing.allocator, .{}); + const alloc = failing.allocator(); + var t = try init(alloc, .{ .cols = 5, .rows = 1 }); + defer t.deinit(alloc); + + try t.pwd.ensureTotalCapacityPrecise(alloc, 3); + failing.fail_index = failing.alloc_index; + try testing.expectError(error.OutOfMemory, t.setPwd("pwd")); + try testing.expect(t.getPwd() == null); +} + /// Set the title for the terminal, as set by escape sequences (e.g. OSC 0/2). pub fn setTitle(self: *Terminal, t: []const u8) !void { self.title.clearRetainingCapacity(); From fdd255c78201ee11da23661b8a2e7b5b5461b09d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 21:41:22 -0700 Subject: [PATCH 19/29] terminal: reserve title terminator atomically setTitle appended the title bytes and terminating NUL with separate fallible operations. If only the terminator allocation failed, it returned OutOfMemory but left a nonempty unterminated buffer that made getTitle panic on its sentinel check. Reserve checked capacity before clearing the existing title, then append both the title and terminator without further allocation. Allocation failure now leaves the prior valid title intact. --- src/terminal/Terminal.zig | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index b79f21bb7..980c7225c 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -3591,11 +3591,18 @@ test "Terminal: setPwd preserves a sentinel on allocation failure" { /// Set the title for the terminal, as set by escape sequences (e.g. OSC 0/2). pub fn setTitle(self: *Terminal, t: []const u8) !void { - self.title.clearRetainingCapacity(); - if (t.len > 0) { - try self.title.appendSlice(self.gpa(), t); - try self.title.append(self.gpa(), 0); + if (t.len == 0) { + self.title.clearRetainingCapacity(); + return; } + + const capacity = std.math.add(usize, t.len, 1) catch + return error.OutOfMemory; + try self.title.ensureTotalCapacity(self.gpa(), capacity); + + self.title.clearRetainingCapacity(); + self.title.appendSliceAssumeCapacity(t); + self.title.appendAssumeCapacity(0); } /// Returns the title for the terminal, if any. The memory is owned by the @@ -3605,6 +3612,18 @@ pub fn getTitle(self: *const Terminal) ?[:0]const u8 { return self.title.items[0 .. self.title.items.len - 1 :0]; } +test "Terminal: setTitle preserves a sentinel on allocation failure" { + var failing = testing.FailingAllocator.init(testing.allocator, .{}); + const alloc = failing.allocator(); + var t = try init(alloc, .{ .cols = 5, .rows = 1 }); + defer t.deinit(alloc); + + try t.title.ensureTotalCapacityPrecise(alloc, 5); + failing.fail_index = failing.alloc_index; + try testing.expectError(error.OutOfMemory, t.setTitle("title")); + try testing.expect(t.getTitle() == null); +} + /// Switch to the given screen type (alternate or primary). /// /// This does NOT handle behaviors such as clearing the screen, From 91f0cf67d51c4bf8c642ed1545583421764ee130 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 21:45:15 -0700 Subject: [PATCH 20/29] terminal: preserve tabstops on resize failure Terminal.resize deinitialized the current tab stops before allocating their replacement. If a resize beyond the inline tab-stop capacity ran out of memory, the terminal retained an undefined tab-stop value and normal deinitialization dereferenced a poisoned pointer. Allocate and initialize the replacement first, then release the old tab stops only after allocation succeeds. A failed resize now retains the original tab stops and remains safe to destroy. --- src/terminal/Terminal.zig | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 980c7225c..0880ee975 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -3518,8 +3518,13 @@ pub fn resize( // Resize our tabstops if (self.cols != cols) { + const tabstops: Tabstops = try .init( + alloc, + cols, + TABSTOP_INTERVAL, + ); self.tabstops.deinit(alloc); - self.tabstops = try .init(alloc, cols, 8); + self.tabstops = tabstops; } // Resize primary screen, which supports reflow @@ -3554,6 +3559,19 @@ pub fn resize( }; } +test "Terminal: resize preserves tabstops on allocation failure" { + var failing = testing.FailingAllocator.init(testing.allocator, .{}); + const alloc = failing.allocator(); + var t = try init(alloc, .{ .cols = 10, .rows = 1 }); + defer t.deinit(alloc); + + failing.fail_index = failing.alloc_index; + try testing.expectError(error.OutOfMemory, t.resize(alloc, 513, 1)); + + try testing.expectEqual(@as(size.CellCountInt, 10), t.cols); + try testing.expect(t.tabstops.get(8)); +} + /// Set the pwd for the terminal. pub fn setPwd(self: *Terminal, pwd: []const u8) !void { if (pwd.len == 0) { From 0c299000f8059d1fe8fa34539a5ed0044c1cfae4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 21:50:33 -0700 Subject: [PATCH 21/29] terminal: preserve aliased selection pins Screen.select accepted an already tracked selection by value. Passing the screen's current selection back into the setter caused the old selection cleanup to free the same pin pair that the replacement retained, so the next selection operation dereferenced stale pool entries and panicked. When replacing tracked state, release only old pins that are not also owned by the replacement. Exact and partial aliases now retain their shared pins while ordinary replacements still reclaim both old entries. --- src/terminal/Screen.zig | 41 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index b47e4e1cd..03710352d 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -2624,8 +2624,28 @@ pub fn select(self: *Screen, sel_: ?Selection) Allocator.Error!void { const tracked_sel = if (sel.tracked()) sel else try sel.track(self); errdefer if (!sel.tracked()) tracked_sel.deinit(self); - // Untrack prior selection - if (self.selection) |*old| old.deinit(self); + // Untrack prior selection pins that aren't also owned by the replacement. + // A caller may pass our current tracked selection back to us by value, so + // releasing both old pins unconditionally would leave the replacement + // pointing at freed pool entries. + if (self.selection) |old| { + const new_bounds = tracked_sel.bounds.tracked; + switch (old.bounds) { + .untracked => old.deinit(self), + .tracked => |old_bounds| { + if (old_bounds.start != new_bounds.start and + old_bounds.start != new_bounds.end) + { + self.pages.untrackPin(old_bounds.start); + } + if (old_bounds.end != new_bounds.start and + old_bounds.end != new_bounds.end) + { + self.pages.untrackPin(old_bounds.end); + } + }, + } + } self.selection = tracked_sel; self.dirty.selection = true; } @@ -8224,6 +8244,23 @@ test "Screen: select replaces existing pins" { try testing.expectEqual(tracked + 2, s.pages.countTrackedPins()); } +test "Screen: reselecting tracked selection preserves its pins" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, .{ .cols = 10, .rows = 2, .max_scrollback = 0 }); + defer s.deinit(); + + try s.select(Selection.init( + s.pages.pin(.{ .active = .{ .x = 1, .y = 0 } }).?, + s.pages.pin(.{ .active = .{ .x = 3, .y = 0 } }).?, + false, + )); + + try s.select(s.selection.?); + try testing.expectEqual(Selection.Order.forward, s.selection.?.order(&s)); +} + test "Screen: selectAll" { const testing = std.testing; const alloc = testing.allocator; From a14a619ceb5d5518155b1e1657f69a3928e9ea9b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 21:52:24 -0700 Subject: [PATCH 22/29] terminal: handle aliased pwd updates setPwd can receive the slice returned by getPwd. Clearing the list retained its allocation, so appending that same slice used memcpy with aliased source and destination ranges and panicked in runtime-safe builds. Resize the list within its reserved capacity and copy the value forward before writing the sentinel. This supports the complete current value and its subslices without weakening allocation-failure atomicity. --- src/terminal/Terminal.zig | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 0880ee975..64b8d9452 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -3583,9 +3583,9 @@ pub fn setPwd(self: *Terminal, pwd: []const u8) !void { return error.OutOfMemory; try self.pwd.ensureTotalCapacity(self.gpa(), capacity); - self.pwd.clearRetainingCapacity(); - self.pwd.appendSliceAssumeCapacity(pwd); - self.pwd.appendAssumeCapacity(0); + self.pwd.items.len = capacity; + std.mem.copyForwards(u8, self.pwd.items[0..pwd.len], pwd); + self.pwd.items[pwd.len] = 0; } /// Returns the pwd for the terminal, if any. The memory is owned by the @@ -3607,6 +3607,15 @@ test "Terminal: setPwd preserves a sentinel on allocation failure" { try testing.expect(t.getPwd() == null); } +test "Terminal: setPwd accepts its current value" { + var t = try init(testing.allocator, .{ .cols = 5, .rows = 1 }); + defer t.deinit(testing.allocator); + + try t.setPwd("file:///tmp"); + try t.setPwd(t.getPwd().?); + try testing.expectEqualStrings("file:///tmp", t.getPwd().?); +} + /// Set the title for the terminal, as set by escape sequences (e.g. OSC 0/2). pub fn setTitle(self: *Terminal, t: []const u8) !void { if (t.len == 0) { From dace6d1c6d4a4351e9abc1d9816ed6dd9cb5a446 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 21:53:38 -0700 Subject: [PATCH 23/29] terminal: handle aliased title updates setTitle can receive the slice returned by getTitle. Clearing the list retained its allocation, so appending that same slice used memcpy with aliased source and destination ranges and panicked in runtime-safe builds. Resize the list within its reserved capacity and copy the value forward before writing the sentinel. This supports the complete current value and its subslices without weakening allocation-failure atomicity. --- src/terminal/Terminal.zig | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 64b8d9452..e5bfd93b7 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -3627,9 +3627,9 @@ pub fn setTitle(self: *Terminal, t: []const u8) !void { return error.OutOfMemory; try self.title.ensureTotalCapacity(self.gpa(), capacity); - self.title.clearRetainingCapacity(); - self.title.appendSliceAssumeCapacity(t); - self.title.appendAssumeCapacity(0); + self.title.items.len = capacity; + std.mem.copyForwards(u8, self.title.items[0..t.len], t); + self.title.items[t.len] = 0; } /// Returns the title for the terminal, if any. The memory is owned by the @@ -3651,6 +3651,15 @@ test "Terminal: setTitle preserves a sentinel on allocation failure" { try testing.expect(t.getTitle() == null); } +test "Terminal: setTitle accepts its current value" { + var t = try init(testing.allocator, .{ .cols = 5, .rows = 1 }); + defer t.deinit(testing.allocator); + + try t.setTitle("Ghostty"); + try t.setTitle(t.getTitle().?); + try testing.expectEqualStrings("Ghostty", t.getTitle().?); +} + /// Switch to the given screen type (alternate or primary). /// /// This does NOT handle behaviors such as clearing the screen, From cfce1cd56c339bd23f92a3434f2b0a7f4573a69b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 21:55:08 -0700 Subject: [PATCH 24/29] terminal: duplicate hyperlinks before replacement startHyperlink accepts borrowed URI and ID slices. When those slices came from the current cursor hyperlink, startHyperlinkOnce ended and freed that hyperlink before duplicating the replacement, then dereferenced the released URI and segfaulted. Duplicate the new hyperlink before ending the prior one. Aliased inputs remain valid through the copy, and allocation failure leaves the existing cursor hyperlink intact. --- src/terminal/Screen.zig | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 03710352d..8798411b0 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -2460,9 +2460,6 @@ fn startHyperlinkOnce( self: *Screen, source: hyperlink.Hyperlink, ) (Allocator.Error || Page.InsertHyperlinkError)!void { - // End any prior hyperlink - self.endHyperlink(); - // Allocate our new Hyperlink entry in non-page memory. This // lets us quickly get access to URI, ID. const link = try self.alloc.create(hyperlink.Hyperlink); @@ -2470,6 +2467,10 @@ fn startHyperlinkOnce( link.* = try source.dupe(self.alloc); errdefer link.deinit(self.alloc); + // End any prior hyperlink only after duplicating the new value. The + // source slices are allowed to reference our current hyperlink. + self.endHyperlink(); + // Insert the hyperlink into page memory var page = self.cursor.page_pin.node.page(); const id: hyperlink.Id = try page.insertHyperlink(link.*); @@ -10033,6 +10034,21 @@ test "Screen: hyperlink start/end" { } } +test "Screen: hyperlink accepts its current values" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, .{ .cols = 5, .rows = 5, .max_scrollback = 0 }); + defer s.deinit(); + + try s.startHyperlink("http://example.com", "current"); + const current = s.cursor.hyperlink.?; + try s.startHyperlink(current.uri, current.id.explicit); + + try testing.expectEqualStrings("http://example.com", s.cursor.hyperlink.?.uri); + try testing.expectEqualStrings("current", s.cursor.hyperlink.?.id.explicit); +} + test "Screen: implicit hyperlink ID wraps" { const testing = std.testing; const alloc = testing.allocator; From d239f98056667bf6053347971b74c36d974f9519 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 22:53:06 -0700 Subject: [PATCH 25/29] terminal/search: drop pruned selections Partial history erasure can remove a page without marking tracked pins as garbage because they move coherently to the next page. ScreenSearch therefore retained a selection whose cached history result was subsequently removed by pruneHistory. Selecting again indexed an empty history result list and panicked. Clear the tracked selection when its combined result index falls within the history suffix being pruned. Retained active and newer history selections keep their existing indices. --- src/terminal/search/screen.zig | 39 ++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/terminal/search/screen.zig b/src/terminal/search/screen.zig index 96f996d59..2fa2e679e 100644 --- a/src/terminal/search/screen.zig +++ b/src/terminal/search/screen.zig @@ -337,6 +337,13 @@ pub const ScreenSearch = struct { // Everything from here forward we assume is invalid because // our history results only get older. const alloc = self.allocator(); + if (self.selected) |*m| { + const first_pruned = self.active_results.items.len + i; + if (m.idx >= first_pruned) { + m.deinit(self.screen); + self.selected = null; + } + } for (self.history_results.items[i..]) |*prune_hl| prune_hl.deinit(alloc); self.history_results.shrinkAndFree(alloc, i); return; @@ -1497,6 +1504,38 @@ test "select after all matches disappear drops the selection" { try testing.expectEqual(0, search.history_results.items.len); } +test "select after partial history erase drops a pruned selection" { + const alloc = testing.allocator; + var t: Terminal = try .init(alloc, .{ + .cols = 10, + .rows = 2, + .max_scrollback = std.math.maxInt(usize), + }); + defer t.deinit(alloc); + const list: *PageList = &t.screens.active.pages; + + var stream = t.vtStream(); + defer stream.deinit(); + + stream.nextSlice("error\r\n"); + const first = list.pages.first.?; + while (list.totalPages() < 3) stream.nextSlice("\r\n"); + const first_rows = first.rows(); + + var search: ScreenSearch = try .init(alloc, t.screens.active, "error"); + defer search.deinit(); + try search.searchAll(); + try testing.expectEqual(0, search.active_results.items.len); + try testing.expectEqual(1, search.history_results.items.len); + try testing.expect(try search.select(.next)); + + list.eraseHistory(.{ .history = .{ .y = first_rows - 1 } }); + try testing.expect(!search.selected.?.highlight.start.garbage); + + try testing.expect(!try search.select(.next)); + try testing.expect(search.selected == null); +} + test "screen search no scrollback has no history" { const alloc = testing.allocator; var t: Terminal = try .init(alloc, .{ From 5d8eb78b73c1f1974319b0ec32c7559dc21b41b0 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 22:58:51 -0700 Subject: [PATCH 26/29] terminal/search: reset pins while feeding PageListSearch.feed changed only the node of its tracked progress pin. If the preceding history page had fewer rows or columns after a split, the retained bottom-right coordinates fell outside that page and the next PageList integrity check panicked. Reset both coordinates to the new node's actual bottom-right cell whenever feed advances. The progress pin now remains valid across heterogeneous page sizes. --- src/terminal/search/pagelist.zig | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/terminal/search/pagelist.zig b/src/terminal/search/pagelist.zig index e8354d799..6907417de 100644 --- a/src/terminal/search/pagelist.zig +++ b/src/terminal/search/pagelist.zig @@ -126,6 +126,9 @@ pub const PageListSearch = struct { // Move our tracked pin to the new node. self.pin.node = node; + self.pin.y = node.rows() - 1; + self.pin.x = node.cols() - 1; + self.pin.garbage = false; if (rem == 0) break; } @@ -510,3 +513,31 @@ test "feed with pruned page" { // Feed should still do nothing try testing.expect(!try search.feed()); } + +test "feed keeps its tracked pin within a shorter page" { + const alloc = testing.allocator; + var pages: PageList = try .init(alloc, 10, 2, null); + defer pages.deinit(); + + const first = pages.pages.first.?; + while (first.rows() < first.capacity().rows) _ = try pages.grow(); + + const second = (try pages.grow()).?; + while (second.rows() < second.capacity().rows) _ = try pages.grow(); + + try pages.split(.{ .node = first, .y = 1, .x = 0 }); + const shorter = first.next.?; + try testing.expect(shorter.rows() < second.rows()); + + var search: PageListSearch = try .init( + alloc, + "x", + &pages, + second, + ); + defer search.deinit(); + + try testing.expect(try search.feed()); + try testing.expectEqual(shorter, search.pin.node); + try testing.expect(pages.pinIsValid(search.pin.*)); +} From 86e1f7b8b53e75809dd8e48d849de502d9397dec Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 23:02:35 -0700 Subject: [PATCH 27/29] terminal/search: reject replaced result pages History pruning only compared cached serials with page_serial_min. Replacing a historical node through compaction left its old serial above that cutoff, so selecting the cached match attempted to track a destroyed node and hit the PageList validity assertion. Validate every flattened chunk by finding the live node and matching its serial without dereferencing stale pointers. Remove only the invalid result and adjust any later selection index so unrelated older results remain available. --- src/terminal/PageList.zig | 18 +++++++++ src/terminal/search/screen.zig | 69 ++++++++++++++++++++++++++++++++-- 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index f5203a809..c9c5f3de1 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -4801,6 +4801,24 @@ pub fn pinIsValid(self: *const PageList, p: Pin) bool { return false; } +/// Returns whether a node pointer and serial still identify the same page in +/// this list. The node pointer is only compared and is safe even if the node +/// has been destroyed or reused since the serial was captured. +pub fn nodeIsValid( + self: *const PageList, + target: *List.Node, + serial: u64, +) bool { + if (serial < self.page_serial_min) return false; + + var it = self.pages.first; + while (it) |node| : (it = node.next) { + if (node == target) return node.serial == serial; + } + + return false; +} + /// Returns the viewport for the given pin, preferring to pin to /// "active" if the pin is within the active area. fn pinIsActive(self: *const PageList, p: Pin) bool { diff --git a/src/terminal/search/screen.zig b/src/terminal/search/screen.zig index 2fa2e679e..6b3a487af 100644 --- a/src/terminal/search/screen.zig +++ b/src/terminal/search/screen.zig @@ -327,11 +327,12 @@ pub const ScreenSearch = struct { fn pruneHistory(self: *ScreenSearch) void { // Go through our history results in order (newest to oldest) to find - // any result that contains an invalid serial. Prune up to that - // point. - for (0..self.history_results.items.len) |i| { + // any result that contains an invalid serial. + var i: usize = 0; + while (i < self.history_results.items.len) { const hl = &self.history_results.items[i]; - const serials = hl.chunks.items(.serial); + const chunks = hl.chunks.slice(); + const serials = chunks.items(.serial); const lowest = serials[0]; if (lowest < self.screen.pages.page_serial_min) { // Everything from here forward we assume is invalid because @@ -348,6 +349,32 @@ pub const ScreenSearch = struct { self.history_results.shrinkAndFree(alloc, i); return; } + + // A page can also be replaced in place, such as by compaction. + // In that case its old serial remains above page_serial_min, so + // validate the captured pointer and serial against the live list. + const nodes = chunks.items(.node); + for (nodes, serials) |node, serial| { + if (!self.screen.pages.nodeIsValid(node, serial)) break; + } else { + i += 1; + continue; + } + + // Only this result is known to be invalid. Older results may live + // on unrelated pages and remain usable, so remove it individually. + const result_idx = self.active_results.items.len + i; + if (self.selected) |*m| { + if (m.idx == result_idx) { + m.deinit(self.screen); + self.selected = null; + } else if (m.idx > result_idx) { + m.idx -= 1; + } + } + + var removed = self.history_results.orderedRemove(i); + removed.deinit(self.allocator()); } } @@ -1536,6 +1563,40 @@ test "select after partial history erase drops a pruned selection" { try testing.expect(search.selected == null); } +test "select after history compaction ignores replaced results" { + const alloc = testing.allocator; + var t: Terminal = try .init(alloc, .{ + .cols = 10, + .rows = 2, + .max_scrollback = std.math.maxInt(usize), + }); + defer t.deinit(alloc); + const list: *PageList = &t.screens.active.pages; + + var stream = t.vtStream(); + defer stream.deinit(); + + stream.nextSlice("error\r\n"); + const first = list.pages.first.?; + while (list.totalPages() < 3) stream.nextSlice("\r\n"); + try list.split(.{ + .node = first, + .y = first.rows() / 2, + .x = 0, + }); + + var search: ScreenSearch = try .init(alloc, t.screens.active, "error"); + defer search.deinit(); + try search.searchAll(); + try testing.expectEqual(1, search.history_results.items.len); + + const replacement = (try list.compact(first)).?; + try testing.expect(replacement != first); + + try testing.expect(!try search.select(.next)); + try testing.expect(search.selected == null); +} + test "screen search no scrollback has no history" { const alloc = testing.allocator; var t: Terminal = try .init(alloc, .{ From 73ac36fa5679f3990b39cdcae75dfd10816e8ee9 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 23:22:26 -0700 Subject: [PATCH 28/29] terminal/search: discard destroyed screen state Search selection could run after the terminal removed a screen but before the next refresh reconciled the cached searchers. Reloading that stale ScreenSearch dereferenced freed PageList nodes, while normal cleanup also tried to untrack pins from the destroyed list. Reconcile under the terminal lock before selecting, track ScreenSet generations so allocator address reuse cannot hide replacements, and release stale search buffers without touching pins already freed with the screen. Cleanup now takes the same lock when live pins must be untracked. --- src/terminal/search/Thread.zig | 100 ++++++++++++++++++++++++++----- src/terminal/search/pagelist.zig | 7 +++ src/terminal/search/screen.zig | 23 ++++++- 3 files changed, 112 insertions(+), 18 deletions(-) diff --git a/src/terminal/search/Thread.zig b/src/terminal/search/Thread.zig index fa09af5f0..d7f0d5625 100644 --- a/src/terminal/search/Thread.zig +++ b/src/terminal/search/Thread.zig @@ -121,7 +121,11 @@ pub fn deinit(self: *Thread) void { // Nothing can possibly access the mailbox anymore, destroy it. self.mailbox.destroy(self.alloc); - if (self.search) |*s| s.deinit(); + if (self.search) |*s| { + self.opts.mutex.lock(); + defer self.opts.mutex.unlock(); + s.deinit(self.opts.terminal); + } } /// The main entrypoint for the thread. @@ -252,11 +256,15 @@ fn drainMailbox(self: *Thread) !void { fn select(self: *Thread, sel: ScreenSearch.Select) !void { const s = if (self.search) |*s| s else return; - const screen_search = s.screens.getPtr(s.last_screen.key) orelse return; self.opts.mutex.lock(); defer self.opts.mutex.unlock(); + // A screen can be removed or replaced between refresh ticks. Reconcile + // while holding the terminal lock before touching any ScreenSearch pins. + s.feed(self.alloc, self.opts.terminal); + const screen_search = s.screens.getPtr(s.last_screen.key) orelse return; + // Make the selection. Ignore the result because we don't // care if the selection didn't change. _ = try screen_search.select(sel); @@ -308,7 +316,11 @@ fn changeNeedle(self: *Thread, needle: []const u8) !void { // If our search is unchanged, do nothing. if (std.ascii.eqlIgnoreCase(s.viewport.needle(), needle)) return; - s.deinit(); + { + self.opts.mutex.lock(); + defer self.opts.mutex.unlock(); + s.deinit(self.opts.terminal); + } self.search = null; // When the search changes then we need to emit that it stopped. @@ -497,6 +509,11 @@ const Search = struct { /// The searchers for all the screens. screens: std.EnumMap(ScreenSet.Key, ScreenSearch), + /// ScreenSet generations captured when each searcher was initialized. + /// Allocators may reuse a destroyed Screen address, so pointer equality + /// alone cannot distinguish replacement screens from stale handles. + screen_generations: std.EnumMap(ScreenSet.Key, usize), + /// All state related to screen switches, collected so that when /// we switch screens it makes everything related stale, too. last_screen: ScreenState, @@ -537,16 +554,35 @@ const Search = struct { return .{ .viewport = vp, .screens = .init(.{}), + .screen_generations = .init(.{}), .last_screen = .{ .key = .primary }, .last_complete = false, .stale_viewport_matches = true, }; } - pub fn deinit(self: *Search) void { + pub fn deinit(self: *Search, t: *Terminal) void { self.viewport.deinit(); var it = self.screens.iterator(); - while (it.next()) |entry| entry.value.deinit(); + while (it.next()) |entry| { + if (self.screenIsValid(&t.screens, entry.key, entry.value)) { + entry.value.deinit(); + } else { + entry.value.deinitScreenInvalid(); + } + } + } + + fn screenIsValid( + self: *const Search, + screens: *const ScreenSet, + key: ScreenSet.Key, + search: *const ScreenSearch, + ) bool { + const generation = self.screen_generations.get(key) orelse return false; + if (generation != screens.generation(key)) return false; + const actual = screens.get(key) orelse return false; + return actual == search.screen; } /// Returns true if all searches on all screens are complete. @@ -629,18 +665,16 @@ const Search = struct { // Remove screens we have that no longer exist or changed. var it = self.screens.iterator(); while (it.next()) |entry| { - const remove: bool = remove: { - // If the screen doesn't exist at all, remove it. - const actual = t.screens.all.get(entry.key) orelse break :remove true; - - // If the screen pointer changed, remove it, the screen - // was totally reinitialized. - break :remove actual != entry.value.screen; - }; + const remove = !self.screenIsValid( + &t.screens, + entry.key, + entry.value, + ); if (remove) { - entry.value.deinit(); + entry.value.deinitScreenInvalid(); _ = self.screens.remove(entry.key); + _ = self.screen_generations.remove(entry.key); } } } @@ -649,7 +683,7 @@ const Search = struct { var it = t.screens.all.iterator(); while (it.next()) |entry| { if (self.screens.contains(entry.key)) continue; - self.screens.put(entry.key, ScreenSearch.init( + const screen_search = ScreenSearch.init( alloc, entry.value.*, self.viewport.needle(), @@ -664,7 +698,12 @@ const Search = struct { ); continue; }, - }); + }; + self.screens.put(entry.key, screen_search); + self.screen_generations.put( + entry.key, + t.screens.generation(entry.key), + ); } } @@ -903,3 +942,32 @@ test { } }, t.screens.active.pages.pointFromPin(.screen, sel.end).?); } } + +test "select after active screen removal" { + const alloc = testing.allocator; + var mutex: std.Thread.Mutex = .{}; + var t: Terminal = try .init(alloc, .{ .cols = 20, .rows = 2 }); + defer t.deinit(alloc); + + _ = try t.switchScreen(.alternate); + + var search: Search = try .init(alloc, "needle"); + search.feed(alloc, &t); + try testing.expectEqual(ScreenSet.Key.alternate, search.last_screen.key); + try testing.expect(search.screens.contains(.alternate)); + + var thread: Thread = undefined; + thread.search = search; + thread.opts = .{ + .mutex = &mutex, + .terminal = &t, + }; + defer if (thread.search) |*active| active.deinit(&t); + + _ = try t.switchScreen(.primary); + t.screens.remove(alloc, .alternate); + + try thread.select(.next); + try testing.expectEqual(ScreenSet.Key.primary, thread.search.?.last_screen.key); + try testing.expect(!thread.search.?.screens.contains(.alternate)); +} diff --git a/src/terminal/search/pagelist.zig b/src/terminal/search/pagelist.zig index 6907417de..98a24c187 100644 --- a/src/terminal/search/pagelist.zig +++ b/src/terminal/search/pagelist.zig @@ -85,6 +85,13 @@ pub const PageListSearch = struct { self.list.untrackPin(self.pin); } + /// Release owned search buffers after the PageList itself has already + /// been destroyed. Its pin belonged to the PageList pool and was freed + /// with that list, so it must not be untracked here. + pub fn deinitListInvalid(self: *PageListSearch) void { + self.window.deinit(); + } + /// Return the next match in the loaded page nodes. If this returns /// null then the PageList search needs to be fed the next node(s). /// Call, `feed` to do this. diff --git a/src/terminal/search/screen.zig b/src/terminal/search/screen.zig index 6b3a487af..448bb98c3 100644 --- a/src/terminal/search/screen.zig +++ b/src/terminal/search/screen.zig @@ -157,10 +157,29 @@ pub const ScreenSearch = struct { } pub fn deinit(self: *ScreenSearch) void { + self.deinitInternal(true); + } + + /// Release owned search state after the underlying Screen has already + /// been destroyed. Tracked pins belonged to that Screen's PageList pool + /// and were freed with it, so only independently owned memory is freed. + pub fn deinitScreenInvalid(self: *ScreenSearch) void { + self.deinitInternal(false); + } + + fn deinitInternal(self: *ScreenSearch, screen_valid: bool) void { const alloc = self.allocator(); self.active.deinit(); - if (self.history) |*h| h.deinit(self.screen); - if (self.selected) |*m| m.deinit(self.screen); + if (self.history) |*h| { + if (screen_valid) { + h.deinit(self.screen); + } else { + h.searcher.deinitListInvalid(); + } + } + if (screen_valid) { + if (self.selected) |*m| m.deinit(self.screen); + } for (self.active_results.items) |*hl| hl.deinit(alloc); self.active_results.deinit(alloc); for (self.history_results.items) |*hl| hl.deinit(alloc); From 9b466e9c5567211543e3f01b73dc76f251a66ccd Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 10 Jul 2026 06:25:15 -0700 Subject: [PATCH 29/29] terminal: fix count-limited page iteration Count-limited PageIterator traversal took the minimum of the page and requested lengths, then tested whether that minimum exceeded the request. The condition was impossible, so iteration never crossed a page. Reverse traversal also excluded the current row and subtracted one from row zero, causing a runtime safety panic. Count the current row in both directions, consume the returned length, and move to an adjacent page only when the request has rows remaining. Returned chunks now preserve their half-open bounds at row zero and across page boundaries. --- src/terminal/PageList.zig | 98 ++++++++++++++++++++++++++++++++------- 1 file changed, 81 insertions(+), 17 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index c9c5f3de1..1ba939f8f 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -5608,18 +5608,15 @@ pub const PageIterator = struct { .count => |*limit| count: { assert(limit.* > 0); // should be handled already - const len = @min(row.node.rows() - row.y, limit.*); - if (len > limit.*) { - self.row = row.down(len); - limit.* -= len; - } else { - self.row = null; - } + const available: usize = row.node.rows() - row.y; + const len = @min(available, limit.*); + limit.* -= len; + self.row = if (limit.* > 0) row.down(len) else null; break :count .{ .node = row.node, .start = row.y, - .end = row.y + len, + .end = @intCast(@as(usize, row.y) + len), }; }, @@ -5677,18 +5674,15 @@ pub const PageIterator = struct { .count => |*limit| count: { assert(limit.* > 0); // should be handled already - const len = @min(row.y, limit.*); - if (len > limit.*) { - self.row = row.up(len); - limit.* -= len; - } else { - self.row = null; - } + const available: usize = @as(usize, row.y) + 1; + const len = @min(available, limit.*); + limit.* -= len; + self.row = if (limit.* > 0) row.up(len) else null; break :count .{ .node = row.node, - .start = row.y - len, - .end = row.y - 1, + .start = @intCast(available - len), + .end = @intCast(available), }; }, @@ -10082,6 +10076,76 @@ test "PageList pageIterator reverse history two pages" { try testing.expect(it.next() == null); } +test "PageList PageIterator reverse count includes row zero" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 2, 2, null); + defer s.deinit(); + + var it: PageIterator = .{ + .row = s.getTopLeft(.screen), + .limit = .{ .count = 1 }, + .direction = .left_up, + }; + const chunk = it.next().?; + try testing.expectEqual(@as(size.CellCountInt, 0), chunk.start); + try testing.expectEqual(@as(size.CellCountInt, 1), chunk.end); + try testing.expect(it.next() == null); +} + +test "PageList PageIterator count crosses page boundaries" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 80, 24, null); + defer s.deinit(); + + const first = s.pages.first.?; + first.page().pauseIntegrityChecks(true); + while (first.rows() < first.capacity().rows) _ = try s.grow(); + first.page().pauseIntegrityChecks(false); + const second = (try s.grow()).?; + + var down: PageIterator = .{ + .row = .{ .node = first, .y = first.rows() - 1 }, + .limit = .{ .count = 2 }, + .direction = .right_down, + }; + { + const chunk = down.next().?; + try testing.expectEqual(first, chunk.node); + try testing.expectEqual(first.rows() - 1, chunk.start); + try testing.expectEqual(first.rows(), chunk.end); + } + { + const chunk = down.next().?; + try testing.expectEqual(second, chunk.node); + try testing.expectEqual(@as(size.CellCountInt, 0), chunk.start); + try testing.expectEqual(@as(size.CellCountInt, 1), chunk.end); + } + try testing.expect(down.next() == null); + + var up: PageIterator = .{ + .row = .{ .node = second }, + .limit = .{ .count = 2 }, + .direction = .left_up, + }; + { + const chunk = up.next().?; + try testing.expectEqual(second, chunk.node); + try testing.expectEqual(@as(size.CellCountInt, 0), chunk.start); + try testing.expectEqual(@as(size.CellCountInt, 1), chunk.end); + } + { + const chunk = up.next().?; + try testing.expectEqual(first, chunk.node); + try testing.expectEqual(first.rows() - 1, chunk.start); + try testing.expectEqual(first.rows(), chunk.end); + } + try testing.expect(up.next() == null); +} + test "PageList cellIterator" { const testing = std.testing; const alloc = testing.allocator;