Commit 3971f2d

Michael Sakaluk  ·  2026-03-18 13:18:02 -0400 EDT
parent 3df5416
fix: two-phase terminal state serialization for nested session cursor corruption

Fixes #31 (cursor position corruption on re-attach over SSH).
Partially addresses #86 (cursor style error after attach).
Contributes to #96 (testing framework).

Problem:
When running nested zmx sessions (zmx→SSH→zmx), re-attaching to the outer
session causes cursor positions and screen content to appear at wrong rows.
The root cause is that serializeTerminalState() writes scrollback + visible
content sequentially. Scrollback lines scroll the terminal before CUP
sequences arrive, shifting all visible content by the scrollback overflow.

Fix:
Split serialization into two phases:
1. Phase 1: Emit scrollback content only (no modes/cursor/keyboard).
   These lines scroll into the terminal's scrollback buffer.
2. Clear visible screen (ESC[2J ESC[H ESC[0m) to reset for phase 2.
3. Phase 2: Emit visible screen only with full extras (modes, cursor,
   keyboard, scrolling region).

The clear between phases ensures visible content starts from a clean slate
regardless of how much scrollback preceded it. Scrollback is preserved in
the terminal's buffer for native scroll-up. This approach is inspired by
tmux's visible-only redraw strategy but preserves terminal scrollback.

Tests:
Added 7 integration tests using ghostty-vt roundtrip verification:
- Cursor position preservation after roundtrip
- CUP-positioned markers survive roundtrip
- Scrollback does not shift visible content (the core bug)
- Nested double-roundtrip (inner→outer→client)
- Alternate screen content not leaked
- Terminal size mismatch (30→24 rows) + roundtrip
- Scrollback + size mismatch + nested roundtrip (stress test)

All tests run in <1 second via `zig build test`.
1 files changed,  +366, -4
+366, -4
......@@ -315,9 +315,67 @@ pub fn serializeTerminalState(alloc: std.mem.Allocator, term: *ghostty_vt.Termin
315315 term.modes.set(.synchronized_output, false);
316316 }
317317
318- var term_formatter = ghostty_vt.formatter.TerminalFormatter.init(term, .vt);
319- term_formatter.content = .{ .selection = null };
320- term_formatter.extra = .{
318+ const pages = &term.screens.active.pages;
319+ const screen_top = pages.getTopLeft(.screen);
320+ const active_top = pages.getTopLeft(.active);
321+ const has_scrollback = !screen_top.eql(active_top);
322+
323+ // Two-phase serialization to preserve scrollback without corrupting
324+ // cursor positions. This matters for nested zmx sessions (zmx→SSH→zmx)
325+ // where the outer daemon's ghostty-vt accumulates inner session scrollback.
326+ //
327+ // Phase 1: Emit scrollback content (plain text with styles, no terminal extras).
328+ // These lines scroll past the visible area into the terminal's scrollback buffer.
329+ // Phase 2: Clear visible screen, then emit visible content with full extras.
330+ // The clear ensures visible content starts from a clean slate regardless of
331+ // how much scrollback preceded it. CUP cursor positioning is then correct.
332+ //
333+ // See: https://github.com/neurosnap/zmx/issues/31
334+
335+ // Phase 1: scrollback only (if any exists)
336+ if (has_scrollback) {
337+ if (active_top.up(1)) |sb_bottom_row| {
338+ var sb_bottom = sb_bottom_row;
339+ sb_bottom.x = @intCast(pages.cols - 1);
340+
341+ var scroll_fmt = ghostty_vt.formatter.TerminalFormatter.init(term, .vt);
342+ scroll_fmt.content = .{ .selection = ghostty_vt.Selection.init(
343+ screen_top,
344+ sb_bottom,
345+ false,
346+ ) };
347+ scroll_fmt.extra = .none; // no modes, cursor, keyboard — just content
348+ scroll_fmt.format(&builder.writer) catch |err| {
349+ std.log.warn("failed to format scrollback err={s}", .{@errorName(err)});
350+ };
351+ }
352+
353+ // Clear visible screen after scrollback. \x1b[2J clears only the visible
354+ // rows (not the scrollback buffer). \x1b[H homes the cursor. \x1b[0m resets
355+ // SGR style so phase 1 styles don't bleed into phase 2.
356+ builder.writer.writeAll("\x1b[2J\x1b[H\x1b[0m") catch {};
357+ }
358+
359+ // Phase 2: visible screen with full extras (modes, cursor, keyboard, etc.)
360+ var vis_fmt = ghostty_vt.formatter.TerminalFormatter.init(term, .vt);
361+
362+ // Restrict content to the active viewport only
363+ const active_tl = pages.pin(.{ .active = .{ .x = 0, .y = 0 } });
364+ const active_br = pages.pin(.{ .active = .{
365+ .x = @intCast(pages.cols - 1),
366+ .y = @intCast(pages.rows - 1),
367+ } });
368+
369+ if (active_tl != null and active_br != null) {
370+ vis_fmt.content = .{ .selection = ghostty_vt.Selection.init(
371+ active_tl.?,
372+ active_br.?,
373+ false,
374+ ) };
375+ }
376+ // Fallback: if pins are somehow invalid, use null selection (all content)
377+
378+ vis_fmt.extra = .{
321379 .palette = false,
322380 .modes = true,
323381 .scrolling_region = true,
......@@ -327,7 +385,7 @@ pub fn serializeTerminalState(alloc: std.mem.Allocator, term: *ghostty_vt.Termin
327385 .screen = .all,
328386 };
329387
330- term_formatter.format(&builder.writer) catch |err| {
388+ vis_fmt.format(&builder.writer) catch |err| {
331389 std.log.warn("failed to format terminal state err={s}", .{@errorName(err)});
332390 return null;
333391 };
......@@ -756,3 +814,307 @@ test "serializeTerminalState excludes synchronized output replay" {
756814 try std.testing.expect(std.mem.indexOf(u8, output, "\x1b[?2004h") != null);
757815 try std.testing.expect(std.mem.indexOf(u8, output, "\x1b[?2026h") == null);
758816 }
817+
818+// ---------------------------------------------------------------------------
819+// Integration tests: serializeTerminalState roundtrip verification
820+//
821+// These tests exercise the ghostty-vt roundtrip pattern:
822+// 1. Create Terminal A, feed VT sequences (scrollback, markers, cursor)
823+// 2. Serialize A via serializeTerminalState()
824+// 3. Create Terminal B (same dimensions), feed serialized bytes
825+// 4. Compare B's screen content and cursor with A's
826+//
827+// This verifies that what the user sees after re-attach matches what the
828+// daemon's terminal state actually contains — including in nested sessions
829+// (zmx→SSH→zmx) where serialized state flows through multiple layers.
830+// ---------------------------------------------------------------------------
831+
832+fn testCreateTerminal(alloc: std.mem.Allocator, cols: u16, rows: u16, vt_data: []const u8) !ghostty_vt.Terminal {
833+ var term = try ghostty_vt.Terminal.init(alloc, .{
834+ .cols = cols,
835+ .rows = rows,
836+ .max_scrollback = 10_000_000,
837+ });
838+ if (vt_data.len > 0) {
839+ var stream = term.vtStream();
840+ defer stream.deinit();
841+ try stream.nextSlice(vt_data);
842+ }
843+ return term;
844+}
845+
846+fn expectScreensMatch(alloc: std.mem.Allocator, expected: *ghostty_vt.Terminal, actual: *ghostty_vt.Terminal) !void {
847+ const exp_str = try expected.plainString(alloc);
848+ defer alloc.free(exp_str);
849+ const act_str = try actual.plainString(alloc);
850+ defer alloc.free(act_str);
851+ try std.testing.expectEqualStrings(exp_str, act_str);
852+}
853+
854+fn expectCursorAt(term: *ghostty_vt.Terminal, row: usize, col: usize) !void {
855+ const cursor = &term.screens.active.cursor;
856+ try std.testing.expectEqual(col, cursor.x);
857+ try std.testing.expectEqual(row, cursor.y);
858+}
859+
860+fn serializeRoundtrip(alloc: std.mem.Allocator, source: *ghostty_vt.Terminal) !ghostty_vt.Terminal {
861+ const serialized = serializeTerminalState(alloc, source) orelse
862+ return error.SerializationFailed;
863+ defer alloc.free(serialized);
864+
865+ var dest = try ghostty_vt.Terminal.init(alloc, .{
866+ .cols = source.screens.active.pages.cols,
867+ .rows = source.screens.active.pages.rows,
868+ .max_scrollback = 10_000_000,
869+ });
870+ var stream = dest.vtStream();
871+ defer stream.deinit();
872+ try stream.nextSlice(serialized);
873+ return dest;
874+}
875+
876+fn expectMarkerAtRow(alloc: std.mem.Allocator, term: *ghostty_vt.Terminal, marker: []const u8, expected_row: usize) !void {
877+ const plain = try term.plainString(alloc);
878+ defer alloc.free(plain);
879+ var row: usize = 0;
880+ var iter = std.mem.splitScalar(u8, plain, '\n');
881+ while (iter.next()) |line| {
882+ if (std.mem.indexOf(u8, line, marker) != null) {
883+ try std.testing.expectEqual(expected_row, row);
884+ return;
885+ }
886+ row += 1;
887+ }
888+ std.debug.print("marker '{s}' not found in terminal output\n", .{marker});
889+ return error.TestExpectedEqual;
890+}
891+
892+test "serializeTerminalState roundtrip preserves cursor position" {
893+ const alloc = std.testing.allocator;
894+
895+ var term = try testCreateTerminal(alloc, 80, 24,
896+ "\x1b[2J" ++ // clear
897+ "\x1b[10;20H" // cursor at row 10, col 20 (1-indexed)
898+ );
899+ defer term.deinit(alloc);
900+
901+ try expectCursorAt(&term, 9, 19); // 0-indexed
902+
903+ var client = try serializeRoundtrip(alloc, &term);
904+ defer client.deinit(alloc);
905+
906+ try expectCursorAt(&client, 9, 19);
907+}
908+
909+test "serializeTerminalState roundtrip preserves CUP-positioned markers" {
910+ const alloc = std.testing.allocator;
911+
912+ var term = try testCreateTerminal(alloc, 80, 24,
913+ "\x1b[2J" ++
914+ "\x1b[2;5HMARK_A" ++
915+ "\x1b[6;15HMARK_B" ++
916+ "\x1b[10;30HMARK_C" ++
917+ "\x1b[14;50HMARK_D" ++
918+ "\x1b[16;20H"
919+ );
920+ defer term.deinit(alloc);
921+
922+ var client = try serializeRoundtrip(alloc, &term);
923+ defer client.deinit(alloc);
924+
925+ try expectScreensMatch(alloc, &term, &client);
926+ try expectMarkerAtRow(alloc, &client, "MARK_A", 1);
927+ try expectMarkerAtRow(alloc, &client, "MARK_B", 5);
928+ try expectMarkerAtRow(alloc, &client, "MARK_C", 9);
929+ try expectMarkerAtRow(alloc, &client, "MARK_D", 13);
930+ try expectCursorAt(&client, 15, 19);
931+}
932+
933+test "serializeTerminalState with scrollback preserves visible content" {
934+ const alloc = std.testing.allocator;
935+
936+ var term = try testCreateTerminal(alloc, 80, 24, "");
937+ defer term.deinit(alloc);
938+
939+ var stream = term.vtStream();
940+ defer stream.deinit();
941+
942+ // Generate 80 lines of scrollback (more than 24 visible rows)
943+ var buf: [32]u8 = undefined;
944+ for (0..80) |i| {
945+ const line = std.fmt.bufPrint(&buf, "SCROLL_{d}\r\n", .{i}) catch unreachable;
946+ try stream.nextSlice(line);
947+ }
948+
949+ // Clear screen and place markers at specific positions
950+ try stream.nextSlice(
951+ "\x1b[2J" ++
952+ "\x1b[2;5HMARK_A" ++
953+ "\x1b[6;15HMARK_B" ++
954+ "\x1b[10;30HMARK_C" ++
955+ "\x1b[16;20H"
956+ );
957+
958+ // Verify source terminal has scrollback
959+ const pages = &term.screens.active.pages;
960+ const has_scrollback = !pages.getTopLeft(.screen).eql(pages.getTopLeft(.active));
961+ try std.testing.expect(has_scrollback);
962+
963+ // Roundtrip: serialize → feed into fresh terminal
964+ var client = try serializeRoundtrip(alloc, &term);
965+ defer client.deinit(alloc);
966+
967+ // Visible content must match (this is the core cursor corruption test)
968+ try expectScreensMatch(alloc, &term, &client);
969+ try expectMarkerAtRow(alloc, &client, "MARK_A", 1);
970+ try expectMarkerAtRow(alloc, &client, "MARK_B", 5);
971+ try expectMarkerAtRow(alloc, &client, "MARK_C", 9);
972+ try expectCursorAt(&client, 15, 19);
973+}
974+
975+test "serializeTerminalState nested roundtrip preserves content" {
976+ // Simulates: inner zmx → serialized state → outer ghostty-vt → serialized again → client
977+ // This is the exact nested session scenario (zmx → SSH → zmx).
978+ const alloc = std.testing.allocator;
979+
980+ // "Inner" terminal with scrollback + markers
981+ var inner = try testCreateTerminal(alloc, 80, 24, "");
982+ defer inner.deinit(alloc);
983+
984+ {
985+ var inner_stream = inner.vtStream();
986+ defer inner_stream.deinit();
987+ var buf: [32]u8 = undefined;
988+ for (0..60) |i| {
989+ const line = std.fmt.bufPrint(&buf, "SCROLL_{d}\r\n", .{i}) catch unreachable;
990+ try inner_stream.nextSlice(line);
991+ }
992+ try inner_stream.nextSlice(
993+ "\x1b[2J" ++
994+ "\x1b[3;10HINNER_A" ++
995+ "\x1b[12;25HINNER_B" ++
996+ "\x1b[20;5H"
997+ );
998+ }
999+
1000+ // Record inner's ground truth
1001+ const inner_cursor_x = inner.screens.active.cursor.x;
1002+ const inner_cursor_y = inner.screens.active.cursor.y;
1003+
1004+ // Serialize inner (simulates inner daemon re-attach to inner client)
1005+ const inner_serialized = serializeTerminalState(alloc, &inner) orelse
1006+ return error.SerializationFailed;
1007+ defer alloc.free(inner_serialized);
1008+
1009+ // "Outer" terminal processes inner's serialized output
1010+ var outer = try testCreateTerminal(alloc, 80, 24, "");
1011+ defer outer.deinit(alloc);
1012+
1013+ {
1014+ var outer_stream = outer.vtStream();
1015+ defer outer_stream.deinit();
1016+ try outer_stream.nextSlice(inner_serialized);
1017+ }
1018+
1019+ // Serialize outer (simulates outer daemon re-attach after detach)
1020+ var client = try serializeRoundtrip(alloc, &outer);
1021+ defer client.deinit(alloc);
1022+
1023+ // Client must see the same content as inner's visible screen
1024+ try expectScreensMatch(alloc, &inner, &client);
1025+ try expectCursorAt(&client, inner_cursor_y, inner_cursor_x);
1026+ try expectMarkerAtRow(alloc, &client, "INNER_A", 2);
1027+ try expectMarkerAtRow(alloc, &client, "INNER_B", 11);
1028+}
1029+
1030+test "serializeTerminalState alternate screen not leaked" {
1031+ const alloc = std.testing.allocator;
1032+
1033+ var term = try testCreateTerminal(alloc, 80, 24,
1034+ "\x1b[?1049h" ++ // enter alt screen
1035+ "\x1b[2J\x1b[3;10HALT_MARK" ++ // write on alt screen
1036+ "\x1b[?1049l" ++ // exit alt screen
1037+ "\x1b[2J\x1b[2;5HMAIN_MARK\x1b[8;20H" // write on main screen
1038+ );
1039+ defer term.deinit(alloc);
1040+
1041+ var client = try serializeRoundtrip(alloc, &term);
1042+ defer client.deinit(alloc);
1043+
1044+ try expectScreensMatch(alloc, &term, &client);
1045+
1046+ const plain = try client.plainString(alloc);
1047+ defer alloc.free(plain);
1048+ try std.testing.expect(std.mem.indexOf(u8, plain, "ALT_MARK") == null);
1049+ try std.testing.expect(std.mem.indexOf(u8, plain, "MAIN_MARK") != null);
1050+}
1051+
1052+test "serializeTerminalState size mismatch roundtrip" {
1053+ const alloc = std.testing.allocator;
1054+
1055+ var term = try testCreateTerminal(alloc, 80, 30,
1056+ "\x1b[2J" ++
1057+ "\x1b[3;10HSIZE_A" ++
1058+ "\x1b[12;20HSIZE_B" ++
1059+ "\x1b[20;40HSIZE_C" ++
1060+ "\x1b[15;15H"
1061+ );
1062+ defer term.deinit(alloc);
1063+
1064+ // Resize to 24 rows (simulates outer terminal being smaller)
1065+ try term.resize(alloc, 80, 24);
1066+
1067+ var client = try serializeRoundtrip(alloc, &term);
1068+ defer client.deinit(alloc);
1069+
1070+ try expectScreensMatch(alloc, &term, &client);
1071+ try expectCursorAt(&client, term.screens.active.cursor.y, term.screens.active.cursor.x);
1072+}
1073+
1074+test "serializeTerminalState scrollback + size mismatch nested roundtrip" {
1075+ const alloc = std.testing.allocator;
1076+
1077+ var inner = try testCreateTerminal(alloc, 80, 30, "");
1078+ defer inner.deinit(alloc);
1079+
1080+ {
1081+ var inner_stream = inner.vtStream();
1082+ defer inner_stream.deinit();
1083+ var buf: [32]u8 = undefined;
1084+ for (0..80) |i| {
1085+ const line = std.fmt.bufPrint(&buf, "LINE_{d}\r\n", .{i}) catch unreachable;
1086+ try inner_stream.nextSlice(line);
1087+ }
1088+ try inner_stream.nextSlice(
1089+ "\x1b[2J" ++
1090+ "\x1b[3;10HSTRESS_A" ++
1091+ "\x1b[12;25HSTRESS_B" ++
1092+ "\x1b[16;20H"
1093+ );
1094+ }
1095+
1096+ // Resize inner to 24 rows (outer terminal is smaller)
1097+ try inner.resize(alloc, 80, 24);
1098+
1099+ const inner_cursor_x = inner.screens.active.cursor.x;
1100+ const inner_cursor_y = inner.screens.active.cursor.y;
1101+
1102+ // Inner serialize → outer processes → outer serialize → client
1103+ const inner_ser = serializeTerminalState(alloc, &inner) orelse
1104+ return error.SerializationFailed;
1105+ defer alloc.free(inner_ser);
1106+
1107+ var outer = try testCreateTerminal(alloc, 80, 24, "");
1108+ defer outer.deinit(alloc);
1109+ {
1110+ var outer_stream = outer.vtStream();
1111+ defer outer_stream.deinit();
1112+ try outer_stream.nextSlice(inner_ser);
1113+ }
1114+
1115+ var client = try serializeRoundtrip(alloc, &outer);
1116+ defer client.deinit(alloc);
1117+
1118+ try expectScreensMatch(alloc, &inner, &client);
1119+ try expectCursorAt(&client, inner_cursor_y, inner_cursor_x);
1120+}