From e43b2d438c35fefaafdd63b582f6fc00cc1166b8 Mon Sep 17 00:00:00 2001 From: Jan Guth Date: Mon, 13 Jul 2026 09:29:28 +0200 Subject: [PATCH] fix(proxy): don't swallow a foreground child's DSR-6n cursor reply MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit atty tracks the cursor with DSR-6n: it sends ESC[6n and strips the terminal's ESC[r;cR reply from stdin. The DsrParser only intercepts while atty has a query outstanding (expecting_reply), but a foreground child that also queries the cursor — atuin's Ctrl+R search is the common case — races with atty's own query. When atty's query is still in flight as the child fires its own, the two replies are byte-identical and atty consumes the one meant for the child, which then hangs ("cursor position could not be read", interactive.rs:1703). Fix: before feeding stdin, if we're expecting a reply but the spawned shell no longer owns the terminal foreground (a child does), abandon our query so `feed` passes the reply straight through to the child. Gated on expecting_reply so the TIOCGPGRP ioctl only runs in the rare arm window, never on ordinary typing; the normal path (shell at its prompt) is byte-identical. DsrParser.abandonQuery clears the gate and flushes any half-buffered cross-chunk sequence to the child. Tests: abandonQuery releases a would-be-claimed reply + flushes a split sequence. All 9 ghost e2e scenarios + debug_capture still pass (atty's own cursor tracking is unaffected — the shell-at-prompt path never hits the gate). Co-Authored-By: Claude Opus 4.8 --- src/cursor_dsr.zig | 15 +++++++++++++++ src/cursor_dsr_tests.zig | 26 ++++++++++++++++++++++++++ src/proxy.zig | 25 +++++++++++++++++++------ 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/src/cursor_dsr.zig b/src/cursor_dsr.zig index 5385cb16..9392ba37 100644 --- a/src/cursor_dsr.zig +++ b/src/cursor_dsr.zig @@ -237,6 +237,21 @@ pub const DsrParser = struct { pub fn markQuerySent(self: *DsrParser) void { self.expecting_reply = true; } + + /// Abandon an outstanding query. A foreground child (e.g. atuin's Ctrl+R + /// search) now owns the terminal, so a `\x1B[…R` on stdin is the answer to + /// ITS `\x1B[6n`, not ours — clear the gate so `feed` passes the reply + /// straight through instead of swallowing it (which strands the child + /// blocked reading its cursor position). Any half-buffered cross-chunk + /// sequence is flushed to `out` so it reaches the child too; returns the + /// number of bytes written. `out` must hold at least `pending_buf.len`. + pub fn abandonQuery(self: *DsrParser, out: []u8) usize { + const n = self.flushPending(out); + self.state = .ground; + self.digits_len = 0; + self.expecting_reply = false; + return n; + } }; /// Strip well-formed CPR replies that atty didn't query for. diff --git a/src/cursor_dsr_tests.zig b/src/cursor_dsr_tests.zig index 9c107c37..4aeadfa9 100644 --- a/src/cursor_dsr_tests.zig +++ b/src/cursor_dsr_tests.zig @@ -387,3 +387,29 @@ test "dropWellFormedCpr: CUP sequence (\\x1b[24;80H) is NOT a CPR — must pass const n = mod.dropWellFormedCpr(input, &out, false); try testing.expectEqualStrings("before\x1B[24;80Hafter", out[0..n]); } + +test "DsrParser: abandonQuery clears the gate so a child's reply passes through" { + var p = DsrParser{}; + p.markQuerySent(); // atty has a query outstanding + var out: [64]u8 = undefined; + // A foreground child (atuin) queried the cursor; before its reply arrives we + // learn it owns the terminal → abandon. Its `\x1B[…R` must NOT be consumed. + _ = p.abandonQuery(&out); + try testing.expect(!p.expecting_reply); + const r = p.feed("\x1B[24;80R", &out); + try testing.expect(r.pos == null); // not claimed + try testing.expectEqualStrings("\x1B[24;80R", out[0..r.filtered_len]); // passed through verbatim +} + +test "DsrParser: abandonQuery flushes a half-buffered cross-chunk sequence" { + var p = DsrParser{}; + p.markQuerySent(); + var out: [64]u8 = undefined; + // Chunk 1: partial reply withheld in pending_buf. + const r1 = p.feed("\x1B[12;", &out); + try testing.expectEqual(@as(usize, 0), r1.filtered_len); // withheld + // Child takes over before chunk 2 → abandon must release the buffered bytes. + const flushed = p.abandonQuery(&out); + try testing.expectEqualStrings("\x1B[12;", out[0..flushed]); + try testing.expect(!p.expecting_reply); +} diff --git a/src/proxy.zig b/src/proxy.zig index 35f0a817..2461c366 100644 --- a/src/proxy.zig +++ b/src/proxy.zig @@ -870,7 +870,20 @@ pub fn run(allocator: std.mem.Allocator, io: std.Io, args: Args) !ExitInfo { // DSR-6n reply intercept — `\x1B[;R` is the // terminal's response to our cursor-position query; // strip it before bash sees it as keyboard input. - const dsr_result = dsr_parser.feed(read_buf[0..read_n], &stdin_filtered_buf); + // + // But a foreground child (e.g. atuin's Ctrl+R search) that + // queried the cursor owns any `\x1B[…R` reply on stdin — if we + // still have a query outstanding we'd swallow the child's reply + // and strand it blocked reading its cursor position. Abandon our + // query so `feed` passes the reply through. Gated on + // expecting_reply so the tcgetpgrp ioctl only runs in the rare + // arm window, never on ordinary typing. + var dsr_pre: usize = 0; + if (dsr_parser.expecting_reply and !shellOwnsForeground(pty.master, child_pid)) { + dsr_pre = dsr_parser.abandonQuery(stdin_filtered_buf[0..]); + } + const dsr_result = dsr_parser.feed(read_buf[0..read_n], stdin_filtered_buf[dsr_pre..]); + const dsr_filtered_len = dsr_pre + dsr_result.filtered_len; if (dsr_result.pos) |pos| { cursor_tracker.setPosition(pos.row, pos.col); trace.log(.cursor, "DSR-6n reply: row={d} col={d}", .{ pos.row, pos.col }); @@ -883,7 +896,7 @@ pub fn run(allocator: std.mem.Allocator, io: std.Io, args: Args) !ExitInfo { // is the answer to ITS OWN cursor query — scrubbing it // strands a program blocked reading the reply. Only the // shell-at-its-prompt case is safe to scrub. - var input: []const u8 = stdin_filtered_buf[0..dsr_result.filtered_len]; + var input: []const u8 = stdin_filtered_buf[0..dsr_filtered_len]; // Only a chunk carrying an ESC can hold a CPR reply — // gate the foreground-pgrp ioctl behind that cheap scan // so ordinary typing never pays for it. @@ -892,12 +905,12 @@ pub fn run(allocator: std.mem.Allocator, io: std.Io, args: Args) !ExitInfo { shellOwnsForeground(pty.master, child_pid)) { const cpr_drop_len = cursor_dsr.dropWellFormedCpr( - stdin_filtered_buf[0..dsr_result.filtered_len], - stdin_filtered_buf[0..dsr_result.filtered_len], + stdin_filtered_buf[0..dsr_filtered_len], + stdin_filtered_buf[0..dsr_filtered_len], false, ); - if (cpr_drop_len < dsr_result.filtered_len) { - trace.log(.cursor, "cpr_scrub: pre={d} post={d} dropped={d}", .{ dsr_result.filtered_len, cpr_drop_len, dsr_result.filtered_len - cpr_drop_len }); + if (cpr_drop_len < dsr_filtered_len) { + trace.log(.cursor, "cpr_scrub: pre={d} post={d} dropped={d}", .{ dsr_filtered_len, cpr_drop_len, dsr_filtered_len - cpr_drop_len }); } input = stdin_filtered_buf[0..cpr_drop_len]; }