From 4ec0543ef41ec6c928b1310e03aa3e17cde87956 Mon Sep 17 00:00:00 2001 From: Jan Guth Date: Thu, 2 Jul 2026 17:31:12 +0200 Subject: [PATCH 1/4] fix(keymap): Alt+Shift+D/W kitty bindings use the unshifted keycode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The kitty keyboard protocol reports a letter key by its UNSHIFTED codepoint plus a shift modifier bit — so Alt+Shift+D is `\x1b[100;4u` (100='d', mod 4=alt+shift), not `\x1b[68;4u` ('D'=68). The debug_capture binding (and the pre-existing security_guard_show_warnings / Alt+Shift+W binding, 87='W') used the uppercase code, so on Ghostty/kitty/foot/WezTerm the CSI-u never matched: the Action never fired and the meta-bytes were dropped by the CSI-u interceptor — pressing Alt+Shift+D produced no feedback at all. (Ctrl+Shift+I at defaults.zig:69 already uses the correct lowercase 105='i'.) Bind the lowercase forms (`\x1b[100;4u`, `\x1b[119;4u`); keep the uppercase ones as a fallback for any terminal that reports the shifted code. Co-Authored-By: Claude Opus 4.8 --- src/defaults.zig | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/defaults.zig b/src/defaults.zig index 310516dd..8c093f78 100644 --- a/src/defaults.zig +++ b/src/defaults.zig @@ -135,20 +135,22 @@ pub const Keymap = struct { .{ .bytes = atty.keymap.key("Alt+i"), .action = .incognito_toggle }, .{ .bytes = "\x1b[105;3u", .action = .incognito_toggle }, .{ .bytes = atty.keymap.key("Ctrl+Shift+D"), .action = .delete_history_match, .label = "Ctrl+Shift+D", .description = "delete the current ghost match from history" }, - // Alt+Shift+W — dump security_guard warn events into - // scrollback + clear the buffer. Dual-encoded: legacy - // terminals emit `\x1b` + uppercase `W` for Alt+Shift+W; - // terminals that push the kitty kbd disambiguate flag - // (Ghostty / kitty / foot / WezTerm) emit `\x1b[87;4u` - // (87 = 'W', modifier 4 = shift+alt). The keymap parser - // doesn't grok `Alt+Shift+` so the raw bytes are - // spelled out here. + // Alt+Shift+W — dump security_guard warn events into scrollback + clear + // the buffer. Legacy terminals emit `\x1b` + uppercase `W`. Kitty-kbd + // terminals (Ghostty / kitty / foot / WezTerm) report the UNSHIFTED + // keycode + a shift modifier → `\x1b[119;4u` (119='w', mod 4=alt+shift); + // 87 ('W') is kept for any terminal that reports the shifted code. .{ .bytes = "\x1bW", .action = .security_guard_show_warnings, .label = "Alt+Shift+W", .description = "dump security_guard warn events to scrollback" }, + .{ .bytes = "\x1b[119;4u", .action = .security_guard_show_warnings }, .{ .bytes = "\x1b[87;4u", .action = .security_guard_show_warnings }, // Alt+Shift+D — capture a debug/feedback report (inert unless - // config.debug.enabled). Legacy `\x1bD` + kitty CSI-u sibling (68='D', - // 4=Alt+Shift), like the other Alt+Shift bindings. + // config.debug.enabled). Legacy `\x1bD` (ESC + shifted letter) + kitty + // kbd CSI-u. Kitty reports the UNSHIFTED keycode plus a shift modifier, + // so Alt+Shift+D is `\x1b[100;4u` (100='d', mod 4=alt+shift) — NOT the + // uppercase 'D'=68 (Ghostty/kitty/foot/WezTerm send the lowercase form). + // 68 is kept too for any terminal that reports the shifted code instead. .{ .bytes = "\x1bD", .action = .debug_capture, .label = "Alt+Shift+D", .description = "capture a debug report (needs config.debug.enabled)" }, + .{ .bytes = "\x1b[100;4u", .action = .debug_capture }, .{ .bytes = "\x1b[68;4u", .action = .debug_capture }, // Alt+P drives the security profile per security_guard. // profile_switch_mode (default .sudo — stages the sudo command). From 22351a76230ab1311057184b1ed802372680a0a2 Mon Sep 17 00:00:00 2001 From: Jan Guth Date: Thu, 2 Jul 2026 17:53:41 +0200 Subject: [PATCH 2/4] feat(debug): surface Alt+Shift+D feedback in the status-bar hint row MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The capture outcome ("report saved → …" / "recording off …" / "save failed") now shows in the status-bar hint row (above the footer, TTL-cleared) instead of being written inline to stdout — where it stacked up across presses and collided with the shell prompt. Falls back to an inline toast only when no status bar is configured. captureDebugReport returns the message; the proxy sets it via sb.setHint + repaints so it appears immediately. The debug_capture e2e now runs with the status bar on to exercise this path. Co-Authored-By: Claude Opus 4.8 --- src/proxy.zig | 43 +++++++++++++++++++----------- tests/e2e/debug_capture/config.zig | 8 ++++-- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/src/proxy.zig b/src/proxy.zig index 35f0a817..146eed4a 100644 --- a/src/proxy.zig +++ b/src/proxy.zig @@ -44,12 +44,15 @@ fn envOrEmpty(name: [*:0]const u8) []const u8 { /// Build a debug report from the recorder + current context and write it to /// disk, printing a one-line toast with the path (or the failure). Runs on the /// `debug_capture` shortcut — off the hot path. +/// Build + save a debug report; returns a short status message (into `out_buf` +/// or a static literal) for the caller to surface via the status-bar hint. fn captureDebugReport( allocator: std.mem.Allocator, r: *const debug_recorder.Recorder, ls: *const LineState, ctx: *const module.Context, -) void { + out_buf: []u8, +) []const u8 { const meta = debug_report.Meta{ .atty_version = atty_version, .cols = ctx.terminal_cols orelse 0, @@ -63,13 +66,10 @@ fn captureDebugReport( .incognito = ctx.incognito, }; const path = debug_report.save(allocator, config.debug.report_dir, meta, r) catch { - writeAll(posix.STDOUT_FILENO, "\r\n[atty debug] report save failed\r\n") catch {}; - return; + return "atty debug: report save failed"; }; defer allocator.free(path); - var buf: [1024]u8 = undefined; - const msg = std.fmt.bufPrint(&buf, "\r\n[atty debug] report saved: {s}\r\n", .{path}) catch return; - writeAll(posix.STDOUT_FILENO, msg) catch {}; + return std.fmt.bufPrint(out_buf, "atty debug: report saved → {s}", .{path}) catch "atty debug: report saved"; } const Pty = @import("pty.zig").Pty; @@ -1278,18 +1278,29 @@ pub fn run(allocator: std.mem.Allocator, io: std.Io, args: Args) !ExitInfo { } }, .debug_capture => { - // Dump the in-memory recorder + context to a report - // file. Swallow the meta-bytes regardless (a bare - // `D` would otherwise echo). Inert with a one-line - // note when the recorder is off. + // Dump the recorder + context to a report file and + // surface the outcome as a status-bar hint (above the + // footer) so it doesn't stack up in scrollback or + // collide with the prompt. Swallow the meta-bytes + // regardless (a bare `D` would otherwise echo). swallow_after_binding = true; - if (debug_rec) |*r| { - captureDebugReport(allocator, r, &line_state, &ctx); - } else if (config.debug.enabled) { - // Enabled but null → Recorder.init failed (OOM). - writeAll(posix.STDOUT_FILENO, "\r\n[atty debug] recorder unavailable (init failed)\r\n") catch {}; + var msg_buf: [640]u8 = undefined; + const msg: []const u8 = if (debug_rec) |*r| + captureDebugReport(allocator, r, &line_state, &ctx, &msg_buf) + else if (config.debug.enabled) + "atty debug: recorder unavailable (init failed)" + else + "atty debug: recording off — set config.debug.enabled"; + if (statusbar) |*sb| { + const ttl: u32 = if (config.statusbar.hint_ttl_ms > 0) config.statusbar.hint_ttl_ms else 4000; + sb.setHint(msg, ttl); + if (!alt_screen.active and !cursor_tracker.inEscape()) + renderStatus(&runtimes, &ctx, sb, &out_buf, incognito_on) catch {}; } else { - writeAll(posix.STDOUT_FILENO, "\r\n[atty debug] recorder off — set config.debug.enabled\r\n") catch {}; + // No status bar → fall back to an inline toast. + var line_buf: [700]u8 = undefined; + const line = std.fmt.bufPrint(&line_buf, "\r\n{s}\r\n", .{msg}) catch "\r\natty debug\r\n"; + writeAll(posix.STDOUT_FILENO, line) catch {}; } }, .llm_exec_toggle_help => { diff --git a/tests/e2e/debug_capture/config.zig b/tests/e2e/debug_capture/config.zig index acf0fbc8..a3b599cc 100644 --- a/tests/e2e/debug_capture/config.zig +++ b/tests/e2e/debug_capture/config.zig @@ -1,9 +1,13 @@ //! debug_capture — atty built with the in-memory debug recorder enabled, so the -//! Alt+Shift+D capture writes a report + prints the toast. report_dir is a temp -//! path so the test doesn't depend on HOME/XDG. +//! Alt+Shift+D capture writes a report + surfaces the outcome. report_dir is a +//! temp path so the test doesn't depend on HOME/XDG. The status bar is on so the +//! scenario exercises the realistic path: the outcome shows in the hint row +//! (above the footer), not inline in scrollback. const atty = @import("atty"); pub const debug: atty.Debug = .{ .enabled = true, .report_dir = "/tmp/atty-e2e-debug", }; + +pub const statusbar: atty.StatusBar = .{ .enabled = true }; From 86dfb44c8a78e10832e0660874ac0ee6b4533eeb Mon Sep 17 00:00:00 2001 From: Jan Guth Date: Thu, 30 Jul 2026 21:34:34 +0200 Subject: [PATCH 3/4] =?UTF-8?q?fix(debug):=20address=20review=20round=201?= =?UTF-8?q?=20=E2=80=94=20hint=20disable,=20CSI-u=20coverage,=20clipping?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot: - Respect `statusbar.hint_ttl_ms == 0` (a documented disable switch) instead of forcing a 4000ms TTL onto it; fall back to the inline toast so an explicit keypress still reports back. - Drop the stale doc paragraph on captureDebugReport that still described the removed inline toast. Subagent (fix-and-ship): - The headline fix had ZERO coverage: the e2e typed the legacy `\x1bD`, so reverting the CSI-u binding still passed. The scenario now exercises BOTH encodings — legacy, then the kitty `\x1b[100;4u` form real terminals send — with a short hint TTL (config) so the row clears between them and the second assertion can't match the first message. Verified it FAILS with the CSI-u binding removed. - Clip the hint message to the terminal width (UTF-8-safe): the hint row paints unclipped, so a long report path wrapped onto the padding row, which the bar never erases — leaving a stale fragment after the TTL expired. - docs/debugging.md: document the hint-row format + the hints-disabled fallback. Not changed here: the "unshifted keycode" rule would be better encoded as an `Alt+Shift+` branch in keymap/parser.zig than as hand-rolled literals (tracked separately). The uppercase `68;4u` / `87;4u` fallbacks stay — harmless, and they cover terminals that report the shifted code. Co-Authored-By: Claude Opus 4.8 --- docs/debugging.md | 8 ++++-- src/proxy.zig | 37 +++++++++++++++++++--------- tests/e2e/debug_capture/config.zig | 5 +++- tests/e2e/debug_capture/scenario.e2e | 13 ++++++++-- 4 files changed, 47 insertions(+), 16 deletions(-) diff --git a/docs/debugging.md b/docs/debugging.md index dca07574..c2422cb3 100644 --- a/docs/debugging.md +++ b/docs/debugging.md @@ -33,12 +33,16 @@ Recompile (`make build`) and run atty as usual. ## Capture When something looks wrong, press **`Alt+Shift+D`**. atty writes a JSON report -to the report directory and prints the path: +to the report directory and shows the path in the status bar's hint row (just +above the footer), so repeated captures don't pile up in your scrollback: ``` -[atty debug] report saved: /home/you/.local/share/atty/reports/report-1751000000-123456789.json +atty debug: report saved -> /home/you/.local/share/atty/reports/report-1751000000-123456789.json ``` +With no status bar configured — or with hints disabled (`statusbar.hint_ttl_ms += 0`) — the same message is printed inline instead. + Nothing is written to disk until you press it — the recorder is a bounded **in-memory** ring, so there is no passive log sitting around to leak. diff --git a/src/proxy.zig b/src/proxy.zig index 146eed4a..0648b73a 100644 --- a/src/proxy.zig +++ b/src/proxy.zig @@ -41,11 +41,19 @@ fn envOrEmpty(name: [*:0]const u8) []const u8 { return ""; } -/// Build a debug report from the recorder + current context and write it to -/// disk, printing a one-line toast with the path (or the failure). Runs on the -/// `debug_capture` shortcut — off the hot path. +/// Truncate to at most `max` bytes without splitting a UTF-8 codepoint. The +/// status-bar hint row paints its text unclipped, so an over-wide message wraps +/// onto the padding row — which the bar never erases, leaving a stale fragment. +fn clipUtf8(s: []const u8, max: usize) []const u8 { + if (s.len <= max) return s; + var n = max; + while (n > 0 and (s[n] & 0xC0) == 0x80) n -= 1; + return s[0..n]; +} + /// Build + save a debug report; returns a short status message (into `out_buf` -/// or a static literal) for the caller to surface via the status-bar hint. +/// or a static literal) for the caller to surface. Runs on the `debug_capture` +/// shortcut — off the hot path. fn captureDebugReport( allocator: std.mem.Allocator, r: *const debug_recorder.Recorder, @@ -1291,13 +1299,20 @@ pub fn run(allocator: std.mem.Allocator, io: std.Io, args: Args) !ExitInfo { "atty debug: recorder unavailable (init failed)" else "atty debug: recording off — set config.debug.enabled"; - if (statusbar) |*sb| { - const ttl: u32 = if (config.statusbar.hint_ttl_ms > 0) config.statusbar.hint_ttl_ms else 4000; - sb.setHint(msg, ttl); - if (!alt_screen.active and !cursor_tracker.inEscape()) - renderStatus(&runtimes, &ctx, sb, &out_buf, incognito_on) catch {}; - } else { - // No status bar → fall back to an inline toast. + // `hint_ttl_ms == 0` means the user disabled the hint + // surface — don't force a TTL onto it; fall back to the + // inline toast so an explicit keypress still reports back. + var shown_in_hint = false; + if (config.statusbar.hint_ttl_ms > 0) { + if (statusbar) |*sb| { + const room: usize = if (ctx.terminal_cols) |c| c else msg.len; + sb.setHint(clipUtf8(msg, room), config.statusbar.hint_ttl_ms); + if (!alt_screen.active and !cursor_tracker.inEscape()) + renderStatus(&runtimes, &ctx, sb, &out_buf, incognito_on) catch {}; + shown_in_hint = true; + } + } + if (!shown_in_hint) { var line_buf: [700]u8 = undefined; const line = std.fmt.bufPrint(&line_buf, "\r\n{s}\r\n", .{msg}) catch "\r\natty debug\r\n"; writeAll(posix.STDOUT_FILENO, line) catch {}; diff --git a/tests/e2e/debug_capture/config.zig b/tests/e2e/debug_capture/config.zig index a3b599cc..7a312f89 100644 --- a/tests/e2e/debug_capture/config.zig +++ b/tests/e2e/debug_capture/config.zig @@ -10,4 +10,7 @@ pub const debug: atty.Debug = .{ .report_dir = "/tmp/atty-e2e-debug", }; -pub const statusbar: atty.StatusBar = .{ .enabled = true }; +// Short hint TTL so the scenario can watch the row clear between the two +// captures — that's how it proves the SECOND (kitty CSI-u) binding fired +// rather than just re-reading the first capture's message. +pub const statusbar: atty.StatusBar = .{ .enabled = true, .hint_ttl_ms = 1500 }; diff --git a/tests/e2e/debug_capture/scenario.e2e b/tests/e2e/debug_capture/scenario.e2e index da5e135d..8dc1c074 100644 --- a/tests/e2e/debug_capture/scenario.e2e +++ b/tests/e2e/debug_capture/scenario.e2e @@ -6,7 +6,7 @@ cols 80 rows 8 -timeout_ms 6000 +timeout_ms 12000 spawn $ATTY bash --norc --noprofile -i wait_for "$" @@ -14,9 +14,18 @@ sleep 200 type "echo hello-debug\r" wait_for "hello-debug" sleep 200 -# Alt+Shift+D (legacy ESC + D) → debug_capture. +# Legacy encoding (ESC + shifted letter). type "\x1bD" wait_for "report saved" +# Let the hint TTL expire so the next assertion can't match this message. +wait_for_absent "report saved" + +# Kitty-keyboard encoding — the form Ghostty/kitty/foot/WezTerm actually send. +# Kitty reports the UNSHIFTED keycode plus a shift bit, so Alt+Shift+D is +# 100 ('d'), not 68 ('D'). Without this step the binding fix has no coverage: +# the legacy form above passes even with the CSI-u binding wrong. +type "\x1b[100;4u" +wait_for "report saved" sleep 100 type "exit\r" exit_code 0 From 1c82b664e4e19874f9128f13c8d40cb2c24fdd2e Mon Sep 17 00:00:00 2001 From: Jan Guth Date: Thu, 30 Jul 2026 21:42:53 +0200 Subject: [PATCH 4/4] =?UTF-8?q?fix(debug):=20address=20review=20round=202?= =?UTF-8?q?=20=E2=80=94=20docs=20accuracy=20+=20hint=20buffer=20bound?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subagent (verdict: ship; it independently mutation-verified the e2e now fails without the CSI-u binding): - CLAUDE.md: the Warn-mode note documented `\x1b[87;4u` as THE kitty encoding for Alt+Shift+W — the very literal that didn't fire. That's the sentence a future agent would copy to reintroduce this bug. Now states the unshifted-keycode rule (119='w') with 87 called out as the fallback. - Bound the hint text by the hint buffer as well as the terminal width: setHint's own clamp is byte-wise and could split a codepoint when cols > hint_buf.len. - docs/debugging.md: show the arrow the code actually emits. Copilot round 2: no new findings. Co-Authored-By: Claude Opus 4.8 --- CLAUDE.md | 2 +- docs/debugging.md | 2 +- src/proxy.zig | 5 ++++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e50a3ac8..0e591692 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -154,7 +154,7 @@ Modules can read `ctx.incognito` to opt into stricter behaviour. By default ghos - **First-paint after activate** of the statusbar should clear the reserved rows (old shell content can leak through DECSTBM otherwise). - **Mouse intercept (`config.mouse.enabled`) is opt-in and DECSET-coupled.** When true, proxy emits `\x1b[?1000h\x1b[?1002h\x1b[?1006h` at startup and the matching disable trio on exit. The intercept is gated on `!shell_owns_input` — once a TUI takes the alt-screen, atty stops parsing CSI-`<` mouse events and lets the raw bytes through so vim/htop/lazygit keep their own mouse handling. `mouse_links` / `mouse_urls` both rely on the proxy delivering the click; no module should DECSET its own mouse stream. - **`mouse_urls` ordering.** Place `mouse_urls` BEFORE `guardrail` in the `modules` tuple. Its `ask_each` banner returns `.swallow` from `onInput` for `y/a/t/Esc/Ctrl-C/Ctrl-U/c`; guardrail's own armed banner also swallows keystrokes. First-match wins via declaration order, so reversing the order lets guardrail eat the URL banner's response keys. -- **Warn-mode UX is render-and-clear.** `Alt+Shift+W` (legacy `\x1bW` + kitty kbd `\x1b[87;4u`) calls `security_guard.onAction(.security_guard_show_warnings)` which dumps the WarnSubscriber buffer to scrollback then calls `sub.clear()`. `clear()` preserves `dropped_total` — that's a session-wide audit counter, not a "you've seen these" indicator. No alt-screen overlay; the dump lives in scrollback. The proxy switch case ALWAYS sets `swallow_after_binding = true` even when dispatch returns false (security_guard not in modules tuple), so the meta bytes never reach readline. +- **Warn-mode UX is render-and-clear.** `Alt+Shift+W` (legacy `\x1bW` + kitty kbd `\x1b[119;4u` — kitty reports the UNSHIFTED keycode, 119=`w`, plus mod 4=alt+shift; `\x1b[87;4u` is kept only as a fallback for terminals that report the shifted code) calls `security_guard.onAction(.security_guard_show_warnings)` which dumps the WarnSubscriber buffer to scrollback then calls `sub.clear()`. `clear()` preserves `dropped_total` — that's a session-wide audit counter, not a "you've seen these" indicator. No alt-screen overlay; the dump lives in scrollback. The proxy switch case ALWAYS sets `swallow_after_binding = true` even when dispatch returns false (security_guard not in modules tuple), so the meta bytes never reach readline. ## Things deliberately not yet built (don't propose without checking with user) diff --git a/docs/debugging.md b/docs/debugging.md index c2422cb3..6f89f7dd 100644 --- a/docs/debugging.md +++ b/docs/debugging.md @@ -37,7 +37,7 @@ to the report directory and shows the path in the status bar's hint row (just above the footer), so repeated captures don't pile up in your scrollback: ``` -atty debug: report saved -> /home/you/.local/share/atty/reports/report-1751000000-123456789.json +atty debug: report saved → /home/you/.local/share/atty/reports/report-1751000000-123456789.json ``` With no status bar configured — or with hints disabled (`statusbar.hint_ttl_ms diff --git a/src/proxy.zig b/src/proxy.zig index 0648b73a..01aa72db 100644 --- a/src/proxy.zig +++ b/src/proxy.zig @@ -1305,7 +1305,10 @@ pub fn run(allocator: std.mem.Allocator, io: std.Io, args: Args) !ExitInfo { var shown_in_hint = false; if (config.statusbar.hint_ttl_ms > 0) { if (statusbar) |*sb| { - const room: usize = if (ctx.terminal_cols) |c| c else msg.len; + // Bound by the hint buffer too — setHint's own + // clamp is byte-wise and would split a codepoint. + const cols: usize = if (ctx.terminal_cols) |c| c else msg.len; + const room = @min(cols, sb.hint_buf.len); sb.setHint(clipUtf8(msg, room), config.statusbar.hint_ttl_ms); if (!alt_screen.active and !cursor_tracker.inEscape()) renderStatus(&runtimes, &ctx, sb, &out_buf, incognito_on) catch {};