diff --git a/src/test/e2e/dsl.zig b/src/test/e2e/dsl.zig index 05efada2..aff6dbda 100644 --- a/src/test/e2e/dsl.zig +++ b/src/test/e2e/dsl.zig @@ -6,6 +6,10 @@ //! rows //! timeout_ms # default 5000, caps wait_for + wait_stable //! env KEY=VALUE +//! dsr_reply on|off # answer DSR-6n cursor queries like a real +//! # terminal (default off — the harness is +//! # otherwise a pure screen scraper). Needed for +//! # anything depending on a cursor round-trip. //! spawn # argv0 is the binary (token-split, no quoting yet) //! # if argv0 is "$ATTY", harness substitutes the binary //! type "string" [pattern] # quoted; \n \r \t \\ \" \xNN supported. @@ -46,6 +50,7 @@ pub const Kind = enum { set_rows, set_timeout_ms, set_env, + dsr_reply, spawn, type_str, key, @@ -174,6 +179,14 @@ pub fn parse(allocator: Allocator, source: []const u8) ParseError!Script { // never emit a sequence the mouse parser would discard. if (col < 1 or col > 65535 or row < 1 or row > 65535) return ParseError.BadInteger; try cmds.append(allocator, .{ .kind = .click, .line = line_no, .int_arg = col, .int_arg2 = row }); + } else if (eq(head, "dsr_reply")) { + // dsr_reply on|off — make the harness answer DSR-6n cursor queries + // like a real terminal. Off by default (the harness is otherwise a + // pure scraper; replying changes what the child reads and would + // churn every existing golden). Needed for cursor-query round-trips. + const on = eq(tail, "on") or eq(tail, "true") or eq(tail, "1"); + if (!on and !(eq(tail, "off") or eq(tail, "false") or eq(tail, "0"))) return ParseError.UnknownDirective; + try cmds.append(allocator, .{ .kind = .dsr_reply, .line = line_no, .int_arg = if (on) 1 else 0 }); } else if (eq(head, "sleep")) { try cmds.append(allocator, .{ .kind = .sleep, .line = line_no, .int_arg = try parseInt(tail) }); } else if (eq(head, "wait_for")) { diff --git a/src/test/e2e/dsl_tests.zig b/src/test/e2e/dsl_tests.zig index 47064b16..5245c889 100644 --- a/src/test/e2e/dsl_tests.zig +++ b/src/test/e2e/dsl_tests.zig @@ -132,3 +132,13 @@ test "DSL string escapes" { defer s.deinit(); try std.testing.expectEqualStrings("a\tb\ncA", s.cmds[0].str_arg); } + +test "DSL dsr_reply parses on/off and rejects anything else" { + var s = try parse(std.testing.allocator, "dsr_reply on\ndsr_reply off\n"); + defer s.deinit(); + try std.testing.expectEqual(@as(usize, 2), s.cmds.len); + try std.testing.expectEqual(Kind.dsr_reply, s.cmds[0].kind); + try std.testing.expectEqual(@as(i64, 1), s.cmds[0].int_arg); + try std.testing.expectEqual(@as(i64, 0), s.cmds[1].int_arg); + try std.testing.expectError(ParseError.UnknownDirective, parse(std.testing.allocator, "dsr_reply maybe\n")); +} diff --git a/src/test/e2e/harness.zig b/src/test/e2e/harness.zig index 4e7921ec..bb78cce2 100644 --- a/src/test/e2e/harness.zig +++ b/src/test/e2e/harness.zig @@ -27,6 +27,10 @@ pub const SpawnOpts = struct { rows: u16, forced_env: []const KV, extra_env: []const KV, + /// Answer DSR-6n cursor queries like a real terminal. Set at construction so + /// it is live before the first pump — a post-spawn assignment would miss + /// atty's startup query if `spawn` ever grew a pump of its own. + dsr_reply: bool = false, }; pub const Session = struct { @@ -40,6 +44,15 @@ pub const Session = struct { text_buf: []u8, exited: bool = false, exit_status: u32 = 0, + /// Answer DSR-6n cursor queries like a real terminal would. Off by default: + /// the harness is otherwise a pure screen scraper, and replying changes what + /// the child receives (so it would churn every existing golden). Opt in per + /// scenario with the `dsr_reply on` verb — required to exercise anything + /// that depends on cursor-query round-trips (atty's own re-anchoring, or a + /// foreground child like atuin querying the cursor). + dsr_reply: bool = false, + /// Bytes of the `\x1b[6n` query matched so far, carried across reads. + dsr_match: usize = 0, pub fn deinit(self: *Session) void { if (!self.exited) self.terminate(); @@ -102,6 +115,64 @@ pub const Session = struct { } } + /// Reply to every DSR-6n (`\x1b[6n`) in `chunk` with the grid's current + /// cursor as `\x1b[;R`, 1-based — what a real terminal sends back. + /// Called after `grid.feed` so the position reflects the chunk just drawn. + /// Whoever queried (atty, or a foreground child like atuin) reads it off the + /// same stdin, which is exactly the contention this models. + fn answerDsr(self: *Session, chunk: []const u8) void { + // Incremental match so a query SPLIT ACROSS READS is still answered — + // `\x1b[6n` can straddle a chunk boundary, and a per-chunk substring + // search would silently miss it (an intermittent, timing-dependent + // no-reply, i.e. a CI flake). + const query = "\x1b[6n"; + for (chunk) |b| { + if (b == query[self.dsr_match]) { + self.dsr_match += 1; + if (self.dsr_match < query.len) continue; + } else { + // Mismatch — restart, but this byte may itself open a new match. + self.dsr_match = if (b == query[0]) 1 else 0; + continue; + } + self.dsr_match = 0; + // Position is the cursor AFTER the whole chunk was fed, not at the + // query byte — so several queries batched into one read all get the + // same answer. Fine for liveness/shape assertions; revisit if a + // scenario ever asserts the coordinates themselves. + var buf: [32]u8 = undefined; + const reply = std.fmt.bufPrint(&buf, "\x1b[{d};{d}R", .{ + self.grid.cur_row + 1, + self.grid.cur_col + 1, + }) catch continue; + // Write straight to the master — NOT writeInput, which drains via + // pumpMs when the buffer is full and would re-enter the pump we're + // called from. Retry INTR/AGAIN rather than dropping the reply: a + // dropped reply strands whoever queried, which surfaces as an + // intermittent scenario timeout. Bounded so a wedged fd can't hang + // the run; EPIPE/EIO (child gone) exits immediately. + var off: usize = 0; + var retries: usize = 0; + while (off < reply.len) { + const rc = std.c.write(self.master, reply[off..].ptr, reply.len - off); + if (rc > 0) { + off += @intCast(rc); + continue; + } + switch (std.posix.errno(rc)) { + .INTR => continue, + .AGAIN => { + retries += 1; + if (retries > 50) break; + var ts = std.c.timespec{ .sec = 0, .nsec = std.time.ns_per_ms }; + _ = std.c.nanosleep(&ts, null); + }, + else => break, + } + } + } + } + /// Pump output for up to `ms` milliseconds. Returns true if we read /// any bytes, false on timeout with nothing. pub fn pumpMs(self: *Session, ms: i32) !bool { @@ -128,6 +199,7 @@ pub const Session = struct { } self.grid.feed(buf[0..r]); try self.cast.record('o', buf[0..r]); + if (self.dsr_reply) self.answerDsr(buf[0..r]); return true; } if (pfd[0].revents & 0x018 != 0) { @@ -245,5 +317,6 @@ pub fn spawn(allocator: Allocator, opts: SpawnOpts) !Session { .grid = grid, .cast = cast, .text_buf = text_buf, + .dsr_reply = opts.dsr_reply, }; } diff --git a/src/test/e2e/runner.zig b/src/test/e2e/runner.zig index ce03da0b..7ed8a35c 100644 --- a/src/test/e2e/runner.zig +++ b/src/test/e2e/runner.zig @@ -307,6 +307,7 @@ fn runScenario(io: std.Io, gpa: Allocator, sc: Scenario, atty_bin: []const u8, u var spawn_argv: []const []const u8 = &.{}; var spawn_seen = false; var first_cmd_after_spawn: usize = 0; + var dsr_reply_on = false; for (script.cmds, 0..) |c, i| { switch (c.kind) { @@ -314,6 +315,11 @@ fn runScenario(io: std.Io, gpa: Allocator, sc: Scenario, atty_bin: []const u8, u .set_rows => rows = @intCast(c.int_arg), .set_timeout_ms => timeout_ms = @intCast(c.int_arg), .set_env => try extra_env.append(gpa, .{ .key = c.str_arg, .value = c.str_arg2 }), + // Only a PRE-spawn directive sets the initial state; one placed after + // `spawn` is a mid-run toggle and is applied by pass 2 instead. + .dsr_reply => if (!spawn_seen) { + dsr_reply_on = c.int_arg == 1; + }, .spawn => { if (spawn_seen) return .{ .fail = "multiple spawn directives" }; spawn_argv = c.argv; @@ -345,6 +351,7 @@ fn runScenario(io: std.Io, gpa: Allocator, sc: Scenario, atty_bin: []const u8, u .rows = rows, .forced_env = &forced_env, .extra_env = extra_env.items, + .dsr_reply = dsr_reply_on, }); defer session.deinit(); @@ -361,6 +368,14 @@ fn runScenario(io: std.Io, gpa: Allocator, sc: Scenario, atty_bin: []const u8, u .set_cols, .set_rows, .set_timeout_ms, .set_env, .spawn => { // Config directives must come before spawn; ignore here. }, + // Also honoured pre-spawn (applied at session creation); allowed + // here so a scenario can toggle the terminal's DSR answering + // mid-run. + .dsr_reply => { + session.dsr_reply = c.int_arg == 1; + // Drop any partial match carried from before the toggle. + session.dsr_match = 0; + }, .type_str => try session.typeWith(c.str_arg, @enumFromInt(c.int_arg)), .key => { const bytes = dsl.keyBytes(c.str_arg) orelse { diff --git a/tests/e2e/dsr_child_reply/config.zig b/tests/e2e/dsr_child_reply/config.zig new file mode 100644 index 00000000..653d7f94 --- /dev/null +++ b/tests/e2e/dsr_child_reply/config.zig @@ -0,0 +1,7 @@ +//! dsr_child_reply — statusbar on so atty actively tracks the cursor with its +//! own DSR-6n queries; that is the state in which a child's cursor reply could +//! plausibly be consumed by atty's interceptor, so it is the interesting one to +//! pin the contract under. +const atty = @import("atty"); + +pub const statusbar: atty.StatusBar = .{ .enabled = true }; diff --git a/tests/e2e/dsr_child_reply/golden/env.toml b/tests/e2e/dsr_child_reply/golden/env.toml new file mode 100644 index 00000000..34c4884d --- /dev/null +++ b/tests/e2e/dsr_child_reply/golden/env.toml @@ -0,0 +1,24 @@ +# atty e2e — recorded environment +atty_version = "0.8.0" +cols = 80 +rows = 24 + +[argv] +0 = "$ATTY" +1 = "bash" +2 = "--norc" +3 = "--noprofile" +4 = "-i" + +[forced_env] +PATH = "/usr/local/bin:/usr/bin:/bin:/sbin:/usr/sbin" +TERM = "xterm-256color" +LANG = "C.UTF-8" +LC_ALL = "C.UTF-8" +HOME = "/tmp" +SHELL = "/bin/sh" +USER = "test" +PS1 = "$ " + +[extra_env] +ATTY_BIN = ".zig-cache/e2e/dsr_child_reply/bin/atty" diff --git a/tests/e2e/dsr_child_reply/scenario.e2e b/tests/e2e/dsr_child_reply/scenario.e2e new file mode 100644 index 00000000..d8234fc5 --- /dev/null +++ b/tests/e2e/dsr_child_reply/scenario.e2e @@ -0,0 +1,45 @@ +# A foreground child that queries the cursor must receive the terminal's DSR-6n +# reply. atty issues its own `ESC[6n` queries to track the prompt and strips the +# `ESC[r;cR` answers off stdin so the shell never sees them as keystrokes — so +# there is a real risk of it consuming a reply that belongs to a child (atuin's +# Ctrl+R search is the motivating case). +# +# `dsr_reply on` makes the harness answer cursor queries like a real terminal. +# Without it no replies exist at all and none of this is observable: the harness +# is otherwise a pure screen scraper. +# +# WHAT THIS GUARDS: "a foreground child's cursor reply is never consumed by +# atty's CPR filter." It passes on today's atty — it is a characterisation test +# pinning that contract against future changes to the DSR interception path, NOT +# a regression test for the atuin hang (still unreproduced, see #553). +# +# The child validates the SHAPE of what it received (`ESC[;`), not just +# that some R-terminated blob arrived — so stray non-CPR bytes can't be mistaken +# for a reply. Note the limit: a CPR that atty leaked is itself CPR-shaped, so +# this separates "a cursor report" from "anything else", not "the child's own +# reply" from "atty's". +# +# The child is a separate `bash -c` (its own process group, so it owns the +# terminal foreground) rather than a builtin, matching how atuin runs. The +# marker is assembled at runtime (`"CURSOR""-$M"`) so the result strings never +# appear in the echoed command line — otherwise the assertions would match the +# echo instead of the child's output. + +cols 80 +rows 24 +timeout_ms 10000 +dsr_reply on + +spawn $ATTY bash --norc --noprofile -i +wait_for "$" +sleep 300 + +type "bash -c 'printf \"\\033[6n\"; re=\"\\[[0-9]+;[0-9]+$\"; IFS= read -s -r -d R -t 3 _r && { [[ $_r =~ $re ]] && M=OK || M=BAD; } || M=LOST; echo \"CURSOR\"\"-$M\"'\r" +wait_for "CURSOR-" +sleep 200 +expect_substr "CURSOR-OK" +expect_no_substr "CURSOR-LOST" +expect_no_substr "CURSOR-BAD" + +type "exit\r" +exit_code 0