a73x

98df7072

test(ptyclient): the terminal owns the mode, and the bare case is pinned

a73x   2026-08-15 17:50

Commit message
test(ptyclient): the terminal owns the mode, and the bare case is pinned

Bracketing lived inside main's verb switch, where no test could reach it —
so the case that matters had no pin: a fixture that bracketed unconditionally
would sail through the e2e paste scenario, because nvim takes valid input
either way, and the suite would report green while the fixture had stopped
modelling a terminal. framePaste is extracted so that case can fail.

The mode moves onto Expecter with it. As file-scope state it was a foot-gun
in a signature: noteModes(bytes) required bytes to be the same growing buffer
its cursor indexed, and a shorter chunk sliced out of bounds from a call site
that looked correct. Expecter owns the buffer, so the misuse is now
unspellable and the tests get a fresh terminal instead of globals to reset.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test/ptyclient.zig
Old New
@@ -13,17 +13,76 @@ const Pty = @import("pty").Pty;
13 const script = @import("script"); 13 const script = @import("script");
14 const decodeEscapes = script.decodeEscapes; 14 const decodeEscapes = script.decodeEscapes;
15 15
16 /// Accumulates everything read off the master and matches needles with 16 const BP_SET = "\x1b[?2004h";
17 /// expect(1) semantics: the search starts at a cursor, and a match 17 const BP_RESET = "\x1b[?2004l";
18 /// advances the cursor past itself. Without the cursor, a needle painted 18
19 /// BEFORE the previous verb would satisfy this one — tp1's post-scroll 19 /// Everything read off the master, which is also everything a real terminal
20 /// expect would pass on bytes from the initial snapshot. 20 /// on the other end of this client would have received. Two things are read
21 /// out of it: needles, with expect(1) semantics — the search starts at a
22 /// cursor and a match advances the cursor past itself, because without that
23 /// a needle painted BEFORE the previous verb would satisfy this one (tp1's
24 /// post-scroll expect would pass on bytes from the initial snapshot) — and
25 /// the bracketed-paste mode, which is what makes `paste` behave the way a
26 /// terminal behaves rather than the way the test wishes it would.
27 ///
28 /// The mode lives HERE rather than at file scope because tracking it needs
29 /// an index into this exact buffer as it grows. As a free function taking
30 /// bytes it had a foot-gun in its signature: hand it a chunk shorter than
31 /// the cursor and the slice is out of bounds, from a call site that looks
32 /// correct. Owning both makes that misuse unspellable, and gives every test
33 /// a fresh terminal instead of globals to reset.
21 const Expecter = struct { 34 const Expecter = struct {
22 buf: std.ArrayList(u8) = .empty, 35 buf: std.ArrayList(u8) = .empty,
23 cursor: usize = 0, 36 cursor: usize = 0,
37 /// Whether the client under test has put this "terminal" into bracketed
38 /// paste — set by the escapes it wrote, never by what a scenario assumes.
39 bracketed_paste: bool = false,
40 /// How far into `buf` noteBracketedPaste has already looked.
41 modes_scanned: usize = 0,
24 42
25 fn feed(self: *Expecter, alloc: std.mem.Allocator, bytes: []const u8) !void { 43 fn feed(self: *Expecter, alloc: std.mem.Allocator, bytes: []const u8) !void {
26 try self.buf.appendSlice(alloc, bytes); 44 try self.buf.appendSlice(alloc, bytes);
45 self.noteBracketedPaste();
46 }
47
48 /// Track the mode over the WHOLE capture rather than one chunk at a time,
49 /// for two reasons a per-chunk scan gets wrong:
50 ///
51 /// * both escapes can land in one read — nvim sets the mode and clears
52 /// it again either side of a shell escape, and 4096 bytes swallow
53 /// both. The mode the terminal is left in is whichever came LAST, not
54 /// whichever `indexOf` the author happened to write second.
55 /// * a read can split the 8-byte escape down the middle, and neither
56 /// half matches anything. Backing the scan up by 7 covers every such
57 /// split, and rescanning bytes is harmless: last-occurrence is
58 /// idempotent.
59 ///
60 /// A window holding neither escape changes nothing: the mode is sticky
61 /// until the client says otherwise, which is what a real terminal does.
62 fn noteBracketedPaste(self: *Expecter) void {
63 const win = self.buf.items[self.modes_scanned -| (BP_SET.len - 1)..];
64 const set = std.mem.lastIndexOf(u8, win, BP_SET);
65 const reset = std.mem.lastIndexOf(u8, win, BP_RESET);
66 if (set) |s| {
67 self.bracketed_paste = if (reset) |r| s > r else true;
68 } else if (reset != null) {
69 self.bracketed_paste = false;
70 }
71 self.modes_scanned = self.buf.items.len;
72 }
73
74 /// A paste as this "terminal" would deliver it: markers only when the
75 /// client has asked for them. Caller owns the result.
76 ///
77 /// Extracted from the verb loop so the NEGATIVE case has somewhere to be
78 /// pinned. Bracketing unconditionally would sail through the e2e paste
79 /// scenario — nvim receives valid input either way — and the suite would
80 /// go on reporting green while the fixture had stopped modelling a
81 /// terminal at all. That is this branch's whole worry, so it gets a test
82 /// rather than a comment.
83 fn framePaste(self: *const Expecter, alloc: std.mem.Allocator, text: []const u8) ![]u8 {
84 if (!self.bracketed_paste) return alloc.dupe(u8, text);
85 return std.mem.concat(alloc, u8, &.{ "\x1b[200~", text, "\x1b[201~" });
27 } 86 }
28 87
29 fn match(self: *Expecter, needle: []const u8) bool { 88 fn match(self: *Expecter, needle: []const u8) bool {
@@ -39,39 +98,6 @@ const Expecter = struct {
39 } 98 }
40 }; 99 };
41 100
42 /// Whether the client under test has put this "terminal" into bracketed
43 /// paste. Read off the same bytes the Expecter accumulates, because that
44 /// buffer is exactly what a real terminal would have received.
45 var bracketed_paste = false;
46 /// How far into that buffer noteModes has already looked.
47 var modes_scanned: usize = 0;
48
49 const BP_SET = "\x1b[?2004h";
50 const BP_RESET = "\x1b[?2004l";
51
52 /// Track the mode over the WHOLE capture rather than one chunk at a time,
53 /// for two reasons a per-chunk scan gets wrong:
54 ///
55 /// * both escapes can land in one read — nvim sets the mode and clears it
56 /// again either side of a shell escape, and 4096 bytes swallow both. The
57 /// mode the terminal is left in is whichever came LAST, not whichever
58 /// `indexOf` the author happened to write second.
59 /// * a read can split the 8-byte escape down the middle, and neither half
60 /// matches anything. Backing the scan up by 7 covers every such split,
61 /// and rescanning bytes is harmless: last-occurrence is idempotent.
62 fn noteModes(buf: []const u8) void {
63 const from = modes_scanned -| (BP_SET.len - 1);
64 const win = buf[from..];
65 const set = std.mem.lastIndexOf(u8, win, BP_SET);
66 const reset = std.mem.lastIndexOf(u8, win, BP_RESET);
67 if (set) |s| {
68 bracketed_paste = if (reset) |r| s > r else true;
69 } else if (reset != null) {
70 bracketed_paste = false;
71 }
72 modes_scanned = buf.len;
73 }
74
75 /// ONE write, asserted: the client's scroll-key parser exact-matches a whole 101 /// ONE write, asserted: the client's scroll-key parser exact-matches a whole
76 /// read, so a short write here would silently turn one keystroke into two. 102 /// read, so a short write here would silently turn one keystroke into two.
77 /// A bracketed paste goes the same way — its markers and its text are one 103 /// A bracketed paste goes the same way — its markers and its text are one
@@ -79,8 +105,11 @@ fn noteModes(buf: []const u8) void {
79 fn writeWhole(master: std.posix.fd_t, bytes: []const u8, verb_no: usize) void { 105 fn writeWhole(master: std.posix.fd_t, bytes: []const u8, verb_no: usize) void {
80 const n = std.posix.write(master, bytes) catch |e| 106 const n = std.posix.write(master, bytes) catch |e|
81 fatal(EXIT_CHILD_DIED, "verb {d}: write to the client's pty failed: {s}", .{ verb_no, @errorName(e) }); 107 fatal(EXIT_CHILD_DIED, "verb {d}: write to the client's pty failed: {s}", .{ verb_no, @errorName(e) });
108 // Named for the payload most likely to hit it: a paste is the one that
109 // grows, and the ceiling is the pty's buffer, not anything this fixture
110 // chose.
82 if (n != bytes.len) 111 if (n != bytes.len)
83 fatal(EXIT_USAGE, "verb {d}: short write ({d} of {d}) — send payloads must fit one write", .{ verb_no, n, bytes.len }); 112 fatal(EXIT_USAGE, "verb {d}: short write ({d} of {d}) — a payload must fit one write into the pty's buffer", .{ verb_no, n, bytes.len });
84 } 113 }
85 114
86 const Verb = union(enum) { 115 const Verb = union(enum) {
@@ -191,7 +220,6 @@ fn drain(alloc: std.mem.Allocator, pty: *Pty, out: std.fs.File, exp: *Expecter)
191 if (n == 0) return false; 220 if (n == 0) return false;
192 try out.writeAll(buf[0..n]); 221 try out.writeAll(buf[0..n]);
193 try exp.feed(alloc, buf[0..n]); 222 try exp.feed(alloc, buf[0..n]);
194 noteModes(exp.buf.items);
195 } 223 }
196 } 224 }
197 225
@@ -291,14 +319,7 @@ pub fn main() !void {
291 switch (verb) { 319 switch (verb) {
292 .send => |bytes| writeWhole(pty.master, bytes, verb_no), 320 .send => |bytes| writeWhole(pty.master, bytes, verb_no),
293 .paste => |text| { 321 .paste => |text| {
294 // Bracket ONLY when this "terminal" has been told to. A 322 const framed = try exp.framePaste(alloc, text);
295 // fixture that always bracketed would pass whether or not
296 // the client ever mirrored the mode, which is the one thing
297 // a paste scenario exists to measure.
298 const framed = if (bracketed_paste)
299 try std.mem.concat(alloc, u8, &.{ "\x1b[200~", text, "\x1b[201~" })
300 else
301 try alloc.dupe(u8, text);
302 defer alloc.free(framed); 323 defer alloc.free(framed);
303 writeWhole(pty.master, framed, verb_no); 324 writeWhole(pty.master, framed, verb_no);
304 }, 325 },
@@ -500,34 +521,62 @@ test "parseLine: paste is a verb and its payload keeps its spaces" {
500 try std.testing.expectEqualStrings("a = 1", v.paste); 521 try std.testing.expectEqualStrings("a = 1", v.paste);
501 } 522 }
502 523
503 test "noteModes: the LAST escape in the window wins, split reads included" { 524 test "Expecter: the LAST bracketed-paste escape wins, and the mode is sticky" {
504 bracketed_paste = false; 525 const alloc = std.testing.allocator;
505 modes_scanned = 0; 526 var e: Expecter = .{};
506 defer { 527 defer e.deinit(alloc);
507 bracketed_paste = false; 528
508 modes_scanned = 0; 529 try std.testing.expect(!e.bracketed_paste);
509 } 530 try e.feed(alloc, "nvim starting \x1b[?2004h");
531 try std.testing.expect(e.bracketed_paste);
532
533 // A read carrying neither escape leaves the mode alone — the client says
534 // when it changes, and silence is not a change.
535 try e.feed(alloc, "rows and rows of ordinary paint");
536 try std.testing.expect(e.bracketed_paste);
537
538 // Both escapes in ONE read, set last: a per-chunk scan whose reset check
539 // happened to be written second would report the opposite.
540 try e.feed(alloc, " shell out \x1b[?2004l and back \x1b[?2004h");
541 try std.testing.expect(e.bracketed_paste);
542
543 try e.feed(alloc, " out \x1b[?2004h again then quit \x1b[?2004l");
544 try std.testing.expect(!e.bracketed_paste);
545 }
546
547 test "Expecter: a set escape split across two reads still registers" {
548 const alloc = std.testing.allocator;
549 var e: Expecter = .{};
550 defer e.deinit(alloc);
551 // The 8-byte escape cut down the middle: neither half matches anything,
552 // so only the backed-up rescan of the second feed can see it.
553 try e.feed(alloc, "paint\x1b[?200");
554 try std.testing.expect(!e.bracketed_paste);
555 try e.feed(alloc, "4hmore paint");
556 try std.testing.expect(e.bracketed_paste);
557 }
558
559 test "Expecter: framePaste brackets only what the client asked to be bracketed" {
560 const alloc = std.testing.allocator;
561 var e: Expecter = .{};
562 defer e.deinit(alloc);
510 563
511 noteModes("nvim starting \x1b[?2004h"); 564 // The negative case, and the one that matters most: a fixture that
512 try std.testing.expect(bracketed_paste); 565 // bracketed unconditionally would still pass the e2e paste scenario,
513 566 // because nvim takes valid input either way. Only this can fail.
514 // Both in one read, set last: a per-chunk scan whose reset check ran 567 const bare = try e.framePaste(alloc, "a = 1");
515 // second would report the opposite. 568 defer alloc.free(bare);
516 noteModes("nvim starting \x1b[?2004h ...shell out \x1b[?2004l back \x1b[?2004h"); 569 try std.testing.expectEqualStrings("a = 1", bare);
517 try std.testing.expect(bracketed_paste); 570
518 571 try e.feed(alloc, BP_SET);
519 noteModes("nvim starting \x1b[?2004h ...shell out \x1b[?2004l back \x1b[?2004h quit \x1b[?2004l"); 572 const framed = try e.framePaste(alloc, "a = 1");
520 try std.testing.expect(!bracketed_paste); 573 defer alloc.free(framed);
521 574 try std.testing.expectEqualStrings("\x1b[200~a = 1\x1b[201~", framed);
522 // Split down the middle of the escape: neither half matches on its own, 575
523 // and only the backed-up rescan sees the whole of it. 576 try e.feed(alloc, BP_RESET);
524 bracketed_paste = false; 577 const bare_again = try e.framePaste(alloc, "a = 1");
525 modes_scanned = 0; 578 defer alloc.free(bare_again);
526 const whole = "abc\x1b[?2004habc"; 579 try std.testing.expectEqualStrings("a = 1", bare_again);
527 noteModes(whole[0 .. whole.len - 5]); // cuts inside the escape
528 try std.testing.expect(!bracketed_paste);
529 noteModes(whole);
530 try std.testing.expect(bracketed_paste);
531 } 580 }
532 581
533 // Forces semantic analysis of every pub decl under `zig build test`, so an 582 // Forces semantic analysis of every pub decl under `zig build test`, so an