diff --git a/internal/session/attach.go b/internal/session/attach.go index 609bf08..6c7a4e2 100644 --- a/internal/session/attach.go +++ b/internal/session/attach.go @@ -36,8 +36,12 @@ const ctrlZ = 0x1a // corrupt the TUI that resumes after detach. // screenEnter opens the attach client's own alternate screen, saving the -// primary screen and cursor underneath. -const screenEnter = "\x1b[?1049h" +// primary screen and cursor underneath, then disables alternate scroll mode +// (?1007, default-on in VTE terminals): on the alt screen with mouse +// reporting off it turns wheel motion into arrow keys typed into the agent. +// The user's setting is saved first (XTSAVE) and restored by screenExit; +// terminals without ?1007 ignore all three sequences. +const screenEnter = "\x1b[?1049h" + "\x1b[?1007s" + "\x1b[?1007l" // screenExit resets every mode the agent could have toggled mid-attach, then // leaves the alternate screen. Terminals ignore sequences they don't @@ -51,6 +55,7 @@ const screenExit = "\x1b[" + // numeric keypad (DECKPNM; DECSTR leaves keypad mode alone) "\x1b(B" + // G0 charset back to ASCII "\x1b[?25h" + // cursor visible + "\x1b[?1007r" + // alternate scroll back to the user's saved setting (XTRESTORE) "\x1b[?1049l" // leave the alt screen: primary buffer and cursor restored // RunAttach is the entry point of `uam __attach`: it puts the terminal in raw @@ -207,6 +212,13 @@ func backDetachEnabled() bool { // byte) until a key that submits or clears the box (Enter, Esc, Ctrl+C, // Ctrl+U). A bare left arrow while the box is believed empty detaches; inside // a draft it keeps moving the cursor. Ctrl+B d always detaches regardless. +// +// Not everything on stdin is a keystroke: agents query the terminal (Ink +// re-requests the cursor position every render) and the replies — CPR, DA1, +// kitty flags, OSC/DCS strings — arrive on the same fd, as do mouse and +// focus events. Terminal-generated traffic never reaches the agent's input +// box, so it is forwarded without touching the estimate (see seqPoisons); +// counting it would wedge the quick detach until the next Enter. type stdinFilter struct { backDetach bool // pendingPrefix is set after Ctrl+B, waiting for the chord's second key. @@ -217,6 +229,14 @@ type stdinFilter struct { typed int // unknown latches when the box may hold text typed cannot account for. unknown bool + // strActive marks an OSC/DCS/SOS/PM/APC string sequence being consumed + // verbatim (a terminal reply to an agent query); strBel allows the OSC + // BEL terminator, strEsc tracks a possible ST (ESC \), and strLen caps + // runaway sequences at maxStrLen. + strActive bool + strBel bool + strEsc bool + strLen int } // boxEmpty reports whether the agent's input box is believed empty. @@ -226,8 +246,13 @@ func (f *stdinFilter) boxEmpty() bool { return !f.unknown && f.typed == 0 } func (f *stdinFilter) clearBox() { f.typed, f.unknown = 0, false } // maxEscLen bounds escape-sequence accumulation; anything longer is flushed -// through verbatim rather than parsed. -const maxEscLen = 8 +// through verbatim rather than parsed. Sized for terminal replies, not just +// keystrokes — a DA1 attribute list runs ~40 bytes. +const maxEscLen = 64 + +// maxStrLen bounds string-sequence (OSC/DCS) consumption the same way; +// color-query and XTGETTCAP replies stay well under it. +const maxStrLen = 4096 // pumpStdin forwards terminal input to the host, filtering the detach chord, // Ctrl+Z, and (when enabled) the left-arrow quick detach. It returns when the @@ -260,6 +285,10 @@ func pumpStdin(stdin io.Reader, conn net.Conn, backDetach bool) { func (f *stdinFilter) filter(chunk []byte) (out []byte, detach bool) { out = make([]byte, 0, len(chunk)+1) for i, b := range chunk { + if f.strActive { + out = f.strByte(out, b) + continue + } if f.pendingPrefix { f.pendingPrefix = false switch b { @@ -329,6 +358,19 @@ func (f *stdinFilter) filter(chunk []byte) (out []byte, detach bool) { // updated forward buffer and whether the left-arrow quick detach fired. func (f *stdinFilter) escByte(out []byte, b byte) ([]byte, bool) { f.esc = append(f.esc, b) + if len(f.esc) == 2 { + switch b { + case ']', 'P', 'X', '^', '_': + // OSC/DCS/SOS/PM/APC: a string sequence — in practice a terminal + // reply to an agent query (OSC 10/11 colors, DCS XTGETTCAP). + // Hand off to strByte, which consumes it through its terminator. + out = append(out, f.esc...) + f.strActive, f.strBel = true, b == ']' + f.strEsc, f.strLen = false, len(f.esc) + f.esc = nil + return out, false + } + } if !escComplete(f.esc) { if len(f.esc) > maxEscLen { out = append(out, f.esc...) @@ -342,14 +384,73 @@ func (f *stdinFilter) escByte(out []byte, b byte) ([]byte, bool) { if f.backDetach && f.boxEmpty() && isLeftArrow(seq) { return out, true } - // Any other navigation may recall history or move through a menu, either - // of which can leave text in the input box — be conservative and require - // a fresh submit/clear before the quick detach re-arms. out = append(out, seq...) - f.unknown = true + if seqPoisons(seq) { + // Navigation may recall history or move through a menu, either of + // which can leave text in the input box — be conservative and require + // a fresh submit/clear before the quick detach re-arms. + f.unknown = true + } return out, false } +// strByte consumes one byte of an in-flight string sequence, forwarding it +// verbatim. The sequence ends at ST (ESC \) or, for OSC only, BEL. Reply +// payloads are not keystrokes, so the input-box estimate stays untouched; a +// sequence exceeding maxStrLen is assumed malformed and poisons it instead. +func (f *stdinFilter) strByte(out []byte, b byte) []byte { + out = append(out, b) + f.strLen++ + switch { + case f.strEsc: + if b == '\\' { // ST: sequence complete + f.strActive, f.strEsc = false, false + return out + } + f.strEsc = b == 0x1b + case b == 0x1b: + f.strEsc = true + case b == 0x07 && f.strBel: // BEL terminates OSC + f.strActive = false + return out + } + if f.strLen > maxStrLen { + f.strActive, f.strEsc = false, false + f.unknown = true + } + return out +} + +// seqPoisons reports whether a completed escape sequence may change the +// agent's input box. Keystrokes (arrows, function keys, alt/meta chords) can +// recall history or navigate menus, so they poison the empty-box estimate; +// terminal replies (cursor position, device attributes, kitty flags, mode +// reports) and terminal events (mouse, focus) never reach the input box and +// stay neutral. +func seqPoisons(seq []byte) bool { + if len(seq) < 3 || seq[1] != '[' { + return true // SS3 keys and alt/meta chords are real input + } + switch seq[2] { + case '<', '?', '>': + // Private-parameter CSI: SGR mouse (CSI < ... M/m), DEC replies + // (CSI ? ... c/u/n, CSI ? ... $y) — none are keystrokes. + return false + } + switch seq[len(seq)-1] { + case 'R', 'c', 'n', 'y', 't', 'I', 'O', 'M': + // CPR, device attributes, status reports, mode/window reports, + // focus in/out, legacy mouse. Known xterm grammar collision: a + // modified F3 (CSI 1;2R) is indistinguishable from a CPR at row 1 + // col 2, and either parameter heuristic misreads common cursor + // positions. CPR wins — Ink agents request it on every render, + // while no supported agent binds modified F3 to text entry, and a + // misfired quick detach leaves the draft intact in the agent. + return false + } + return true +} + // escComplete reports whether esc (starting with ESC, len >= 2) is a full // sequence: CSI (ESC [ ... final 0x40–0x7e), SS3 (ESC O x), or a two-byte // alt/meta escape. diff --git a/internal/session/attach_filter_test.go b/internal/session/attach_filter_test.go index 576598e..3a7a6af 100644 --- a/internal/session/attach_filter_test.go +++ b/internal/session/attach_filter_test.go @@ -190,3 +190,108 @@ func TestTabDisarmsUntilClear(t *testing.T) { t.Fatal("Ctrl+U after a tab should re-arm") } } + +// Agents query the terminal (Ink-based ones request the cursor position on +// every render) and the replies arrive on stdin mixed with real keystrokes. +// Replies never reach the agent's input box, so they must pass through +// without disarming the left-arrow quick detach. +func TestTerminalRepliesDoNotDisarm(t *testing.T) { + replies := []string{ + "\x1b[24;80R", // CPR: cursor position report + "\x1b[?64;1;2;6;9;15;18;21;22c", // DA1: primary device attributes + "\x1b[?1u", // kitty keyboard flags report + "\x1b[0n", // DSR: terminal OK + "\x1b[?2004;1$y", // DECRPM: mode report + } + for _, reply := range replies { + f := &stdinFilter{backDetach: true} + out, detach := runFilter(t, f, reply) + if detach || out != reply { + t.Fatalf("reply %q must pass through untouched, out=%q detach=%v", reply, out, detach) + } + if _, detach := runFilter(t, f, "\x1b[D"); !detach { + t.Fatalf("left arrow after reply %q should still detach", reply) + } + } +} + +// OSC and DCS replies (color queries, XTGETTCAP) carry free-form payloads; +// counting those bytes as typed runes would wedge the quick detach. +func TestStringRepliesDoNotDisarm(t *testing.T) { + replies := []string{ + "\x1b]11;rgb:1e1e/1e1e/1e1e\x1b\\", // OSC color reply, ST-terminated + "\x1b]10;rgb:ffff/ffff/ffff\x07", // OSC color reply, BEL-terminated + "\x1bP1+r524742=38\x1b\\", // DCS XTGETTCAP reply + } + for _, reply := range replies { + f := &stdinFilter{backDetach: true} + out, detach := runFilter(t, f, reply) + if detach || out != reply { + t.Fatalf("string reply %q must pass through untouched, out=%q detach=%v", reply, out, detach) + } + if _, detach := runFilter(t, f, "\x1b[D"); !detach { + t.Fatalf("left arrow after string reply %q should still detach", reply) + } + } +} + +func TestStringReplySplitAcrossReads(t *testing.T) { + f := &stdinFilter{backDetach: true} + out, detach := runFilter(t, f, "\x1b]11;rgb:1e", "1e/1e1e/1e1e\x1b", "\\", "\x1b[D") + if !detach { + t.Fatal("left arrow after a split string reply should still detach") + } + if out != "\x1b]11;rgb:1e1e/1e1e/1e1e\x1b\\" { + t.Fatalf("split reply must be forwarded intact, out=%q", out) + } +} + +func TestMouseEventsDoNotDisarm(t *testing.T) { + f := &stdinFilter{backDetach: true} + wheel := "\x1b[<64;10;20M\x1b[<65;10;20m" // SGR wheel press + release + out, detach := runFilter(t, f, wheel) + if detach || out != wheel { + t.Fatalf("mouse events must pass through untouched, out=%q detach=%v", out, detach) + } + if _, detach := runFilter(t, f, "\x1b[D"); !detach { + t.Fatal("left arrow after mouse events should still detach") + } +} + +func TestFocusEventsDoNotDisarm(t *testing.T) { + f := &stdinFilter{backDetach: true} + out, detach := runFilter(t, f, "\x1b[I\x1b[O") // focus in + out + if detach || out != "\x1b[I\x1b[O" { + t.Fatalf("focus events must pass through untouched, out=%q detach=%v", out, detach) + } + if _, detach := runFilter(t, f, "\x1b[D"); !detach { + t.Fatal("left arrow after focus events should still detach") + } +} + +// Real navigation keys still poison the estimate even with replies neutral: +// only terminal-generated traffic is exempt. +func TestArrowAndFunctionKeysStillDisarm(t *testing.T) { + for _, key := range []string{"\x1b[A", "\x1b[B", "\x1b[Z", "\x1bOP", "\x1bf"} { + f := &stdinFilter{backDetach: true} + if _, detach := runFilter(t, f, key, "\x1b[D"); detach { + t.Fatalf("left arrow after key %q must not detach", key) + } + } +} + +// Deliberate trade-off, pinned: xterm's modified F3 (CSI 1;2R) shares its +// grammar with a cursor position report at row 1 col 2, and no parameter +// heuristic separates them without misreading common cursor positions. The +// filter sides with CPR (constant Ink traffic) over modified F3 (bound to +// text entry by no supported agent) — see seqPoisons. +func TestModifiedF3ReadsAsCursorReply(t *testing.T) { + f := &stdinFilter{backDetach: true} + out, detach := runFilter(t, f, "\x1b[1;2R") + if detach || out != "\x1b[1;2R" { + t.Fatalf("CSI 1;2R must pass through untouched, out=%q detach=%v", out, detach) + } + if _, detach := runFilter(t, f, "\x1b[D"); !detach { + t.Fatal("left arrow after a CPR-shaped sequence should still detach") + } +} diff --git a/internal/session/session_test.go b/internal/session/session_test.go index 04e068e..457df81 100644 --- a/internal/session/session_test.go +++ b/internal/session/session_test.go @@ -402,6 +402,11 @@ func TestAttachOwnsTerminalStateOnTTY(t *testing.T) { if clear := strings.Index(pre, "\x1b[2J"); clear >= 0 && clear < enter { t.Fatalf("replay clear must land inside the alt screen, not on the primary: %q", pre) } + // Alternate scroll mode (?1007) turns mouse wheel motion into arrow keys + // on the alt screen; left enabled, scrolling types into the agent. + if scroll := strings.Index(pre, "\x1b[?1007l"); scroll < 0 || scroll < enter { + t.Fatalf("attach must disable alternate scroll inside its alt screen: %q", pre) + } if _, err := ptmx.Write([]byte{0x02, 'd'}); err != nil { // Ctrl+B d t.Fatal(err) @@ -424,6 +429,7 @@ func TestAttachOwnsTerminalStateOnTTY(t *testing.T) { "\x1b[?1000;1002;1003;1004;1005;1006;1015l", // mouse tracking + focus reporting off "\x1b[?2004l", // bracketed paste off "\x1b[?25h", // cursor visible + "\x1b[?1007r", // alternate scroll restored to the user's saved setting } { idx := strings.Index(full, reset) if idx < 0 {