diff --git a/internal/session/ssh.go b/internal/session/ssh.go index 4665a386..cee2f198 100644 --- a/internal/session/ssh.go +++ b/internal/session/ssh.go @@ -337,11 +337,35 @@ func (r *SSHRunner) Attach(sessionID string) error { }() // Read stdin, intercept Ctrl+Q (all encodings), forward the rest. + // + // stdinReaderDone closes when this goroutine returns, and stdinReaderStop + // tells it to. Both are required because the remote process can exit on its + // own (the <-cmdDone branch below), and on that path nothing pressed Ctrl+Q + // — without a stop signal the reader stays parked in a blocking + // os.Stdin.Read that closing the PTY cannot interrupt. It would then be + // queued on the same tty as Bubble Tea's reader when Attach returns, win the + // next keystroke on FIFO wakeup order, and swallow it. Same defect and same + // fix as the local tmux attach path (internal/tmux.attachStdinPump). + stdinReaderDone := make(chan struct{}) + stdinReaderStop := make(chan struct{}) wg.Add(1) go func() { defer wg.Done() + defer close(stdinReaderDone) buf := make([]byte, 256) + fd := int(os.Stdin.Fd()) // #nosec G115 -- an OS file descriptor is a small positive int for { + // Poll before reading so the stop signal is observable; a blocking + // read on a tty inherited from the shell is not interruptible. + select { + case <-stdinReaderStop: + return + default: + } + if !tmux.PollFdReady(fd, tmux.AttachStdinPollInterval) { + continue + } + n, err := os.Stdin.Read(buf) if err != nil { break @@ -385,7 +409,19 @@ func (r *SSHRunner) Attach(sessionID string) error { case <-outputDone: case <-time.After(50 * time.Millisecond): } - termreply.QuarantineFor(sshAttachReplyQuarantine) + // Hand stdin back to the TUI: stop the reader, then drop whatever the + // remote's teardown left in the input queue and arm the reply quarantine. + // The join-before-flush ordering is the load-bearing invariant here, so this + // calls the same tmux.QuiesceAttachInput the local attach path uses rather + // than re-implementing it — that function's mutation-checked tests are what + // protect the ordering, and an inline copy here would inherit none of them. + close(stdinReaderStop) + tmux.QuiesceAttachInput( + stdinReaderDone, + tmux.AttachStdinReaderStopTimeout, + func() { _ = tmux.FlushInput(int(os.Stdin.Fd())) }, // #nosec G115 -- fd is a small positive int + func() { termreply.QuarantineFor(sshAttachReplyQuarantine) }, + ) // Reset terminal styles that may have leaked from the remote session. _, _ = os.Stdout.WriteString("\x1b]8;;\x1b\\\x1b[0m\x1b[24m\x1b[39m\x1b[49m") diff --git a/internal/tmux/attach_stdin_pump_test.go b/internal/tmux/attach_stdin_pump_test.go new file mode 100644 index 00000000..e33b70c0 --- /dev/null +++ b/internal/tmux/attach_stdin_pump_test.go @@ -0,0 +1,311 @@ +//go:build !windows +// +build !windows + +package tmux + +import ( + "bytes" + "context" + "os" + "sync" + "testing" + "time" +) + +// newTestPump wires a pump to an os.Pipe standing in for the host tty. The +// pipe is enough because run() only needs a pollable fd — it never puts the +// descriptor into raw mode itself. +func newTestPump(t *testing.T, opts AttachOptions) (*attachStdinPump, *os.File, *bytes.Buffer) { + t.Helper() + + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe: %v", err) + } + t.Cleanup(func() { + _ = r.Close() + _ = w.Close() + }) + + out := &bytes.Buffer{} + detach := opts.DetachByte + if detach == 0 { + detach = 17 // Ctrl+Q + } + return &attachStdinPump{ + in: r, + out: out, + detach: detach, + opts: opts, + // Backdate so the reply-quarantine filter is disarmed and plain input + // passes through verbatim; the armed path has its own coverage. + startTime: time.Now().Add(-time.Hour), + ioErrors: make(chan error, 2), + }, w, out +} + +type pumpResult struct { + outcome SwitchIntent + interrupted bool +} + +func runPump(ctx context.Context, p *attachStdinPump) <-chan pumpResult { + done := make(chan pumpResult, 1) + go func() { + outcome, interrupted := p.run(ctx) + done <- pumpResult{outcome: outcome, interrupted: interrupted} + }() + return done +} + +// TestAttachStdinPump_StopsOnContextCancel is the regression test for the +// stolen-keystroke bug: when a session's pane process exits on its own, nothing +// pressed the detach key, so the stdin reader had no reason to stop. It stayed +// parked in a blocking os.Stdin.Read that cancel() could not interrupt, and +// AttachWithOptions returned anyway. Back on the deck, the stale reader and +// Bubble Tea's reader were both queued on the same tty; the stale one had +// waited longer, so it won the next keystroke, forwarded it to the closed PTY +// and died — the user's first keypress after a session ended did nothing. +func TestAttachStdinPump_StopsOnContextCancel(t *testing.T) { + pump, w, _ := newTestPump(t, AttachOptions{}) + + ctx, cancel := context.WithCancel(context.Background()) + done := runPump(ctx, pump) + + // Let the pump settle into its poll loop with no input available. + time.Sleep(2 * AttachStdinPollInterval) + select { + case res := <-done: + t.Fatalf("pump exited before cancel: %+v", res) + default: + } + + cancelledAt := time.Now() + cancel() + + // The bound must stay UNDER AttachStdinReaderStopTimeout, not merely under + // some generous ceiling. cleanupAttach gives up waiting at that backstop and + // returns anyway, so a pump that exits slower than it is still broken in + // production - the reader outlives the attach and eats a keypress - while a + // test that allowed backstop+1s would happily pass. Assert the invariant + // cleanupAttach actually depends on: the pump exits before the backstop fires. + select { + case res := <-done: + if elapsed := time.Since(cancelledAt); elapsed >= AttachStdinReaderStopTimeout { + t.Errorf("pump took %v to exit, want < %v (cleanupAttach's backstop); "+ + "at or beyond it the reader outlives the attach", + elapsed, AttachStdinReaderStopTimeout) + } + if res.interrupted { + t.Errorf("interrupted = true on a context cancel, want false") + } + if res.outcome != SwitchNone { + t.Errorf("outcome = %v, want SwitchNone", res.outcome) + } + case <-time.After(AttachStdinReaderStopTimeout): + t.Fatal("pump did not exit before cleanupAttach's backstop; a surviving " + + "reader swallows the first keystroke on return to the deck") + } + + // The pump is gone, so a keystroke arriving now must be left for the next + // reader rather than consumed and dropped. + if _, err := w.Write([]byte("x")); err != nil { + t.Fatalf("write after cancel: %v", err) + } + if !PollFdReady(int(pump.in.Fd()), time.Second) { + t.Fatal("keystroke written after the pump exited was consumed by it") + } + buf := make([]byte, 1) + if _, err := pump.in.Read(buf); err != nil { + t.Fatalf("read back: %v", err) + } + if buf[0] != 'x' { + t.Errorf("read back %q, want %q", buf[0], 'x') + } +} + +func TestAttachStdinPump_ForwardsInputAndInterceptsDetach(t *testing.T) { + pump, w, out := newTestPump(t, AttachOptions{DetachByte: 17}) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + done := runPump(ctx, pump) + + if _, err := w.Write([]byte("hello\x11")); err != nil { + t.Fatalf("write: %v", err) + } + + select { + case res := <-done: + if !res.interrupted { + t.Error("interrupted = false on the detach key, want true") + } + if res.outcome != SwitchNone { + t.Errorf("outcome = %v, want SwitchNone for a plain detach", res.outcome) + } + case <-time.After(5 * time.Second): + t.Fatal("pump did not exit on the detach key") + } + + // Bytes ahead of the detach key are forwarded; the detach key itself is not. + if got := out.String(); got != "hello" { + t.Errorf("forwarded %q, want %q", got, "hello") + } +} + +func TestAttachStdinPump_ReportsSwitchIntent(t *testing.T) { + pump, w, _ := newTestPump(t, AttachOptions{DetachByte: 17, SwitchKeyByte: 19}) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + done := runPump(ctx, pump) + + if _, err := w.Write([]byte{19}); err != nil { + t.Fatalf("write: %v", err) + } + + select { + case res := <-done: + if !res.interrupted { + t.Error("interrupted = false on the switch key, want true") + } + if res.outcome != SwitchRequested { + t.Errorf("outcome = %v, want SwitchRequested", res.outcome) + } + case <-time.After(5 * time.Second): + t.Fatal("pump did not exit on the switch key") + } +} + +// TestAttachStdinPump_StopsOnEOF covers the closed-stdin path: the pump must +// report a non-interrupt exit rather than spinning on a dead descriptor. +func TestAttachStdinPump_StopsOnEOF(t *testing.T) { + pump, w, _ := newTestPump(t, AttachOptions{}) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + done := runPump(ctx, pump) + + if err := w.Close(); err != nil { + t.Fatalf("close write end: %v", err) + } + + select { + case res := <-done: + if res.interrupted { + t.Error("interrupted = true on EOF, want false") + } + case <-time.After(5 * time.Second): + t.Fatal("pump did not exit on stdin EOF") + } +} + +// --- QuiesceAttachInput: the teardown half of the fix --- +// +// These pin the two invariants cleanupAttach depends on and that the pump tests +// above cannot see: that the stdin reader is joined BEFORE the input queue is +// flushed, and that the flush runs on every exit path rather than only after a +// detach keypress. + +func TestQuiesceAttachInput_WaitsForReaderBeforeFlushing(t *testing.T) { + readerDone := make(chan struct{}) + + var mu sync.Mutex + var flushedWhileReaderAlive bool + var flushed, quarantined bool + readerAlive := true + + flush := func() { + mu.Lock() + defer mu.Unlock() + flushed = true + if readerAlive { + // Flushing here would discard the capability replies the reader is + // about to consume anyway, and worse, leave the reader running to + // eat a real keystroke after the queue was cleared. + flushedWhileReaderAlive = true + } + } + quarantine := func() { + mu.Lock() + defer mu.Unlock() + quarantined = true + } + + // Release the reader only after quiesce has had time to block on it. + go func() { + time.Sleep(50 * time.Millisecond) + mu.Lock() + readerAlive = false + mu.Unlock() + close(readerDone) + }() + + exited := QuiesceAttachInput(readerDone, AttachStdinReaderStopTimeout, flush, quarantine) + + if !exited { + t.Error("exited = false, want true (the reader finished well inside the backstop)") + } + mu.Lock() + defer mu.Unlock() + if flushedWhileReaderAlive { + t.Error("flushed the input queue while the stdin reader was still running") + } + if !flushed { + t.Error("flush was never called") + } + if !quarantined { + t.Error("quarantine was never armed") + } +} + +// TestQuiesceAttachInput_FlushesOnEveryExitPath guards the removal of the old +// `if didDetach` gate. The flush used to run only when the user pressed the +// detach key; on the process-exit path the stale reader incidentally swallowed +// the terminal's capability replies instead. With the reader now stopping +// cleanly, skipping the flush here would let those bytes reach the TUI. +func TestQuiesceAttachInput_FlushesOnEveryExitPath(t *testing.T) { + readerDone := make(chan struct{}) + close(readerDone) // reader already gone, as on a process-exit teardown + + flushed, quarantined := false, false + exited := QuiesceAttachInput(readerDone, AttachStdinReaderStopTimeout, + func() { flushed = true }, + func() { quarantined = true }, + ) + + if !exited { + t.Error("exited = false, want true") + } + if !flushed || !quarantined { + t.Errorf("flushed=%v quarantined=%v, want both true on every exit path", + flushed, quarantined) + } +} + +// TestQuiesceAttachInput_BackstopFires covers the wedged-reader case: quiesce +// must give up rather than hang the TUI forever, and must still flush. +func TestQuiesceAttachInput_BackstopFires(t *testing.T) { + neverDone := make(chan struct{}) // reader wedged in an uninterruptible read + + flushed := false + start := time.Now() + exited := QuiesceAttachInput(neverDone, 100*time.Millisecond, + func() { flushed = true }, + func() {}, + ) + elapsed := time.Since(start) + + if exited { + t.Error("exited = true, want false (the reader never finished)") + } + if !flushed { + t.Error("flush must still run after the backstop fires") + } + if elapsed < 100*time.Millisecond { + t.Errorf("returned after %v, want >= the 100ms backstop", elapsed) + } + if elapsed > time.Second { + t.Errorf("returned after %v, far past the backstop", elapsed) + } +} diff --git a/internal/tmux/pty.go b/internal/tmux/pty.go index ab45cff7..1ea47337 100644 --- a/internal/tmux/pty.go +++ b/internal/tmux/pty.go @@ -17,6 +17,7 @@ import ( "github.com/asheshgoplani/agent-deck/internal/termreply" "github.com/creack/pty" + "golang.org/x/sys/unix" "golang.org/x/term" ) @@ -30,6 +31,41 @@ const attachOutputDrainTimeout = 250 * time.Millisecond // an attached session. const attachReplyQuarantine = 500 * time.Millisecond +// AttachStdinPollInterval bounds how long the stdin reader may sit inside +// poll(2) before it re-checks the attach context. A blocking os.Stdin.Read +// cannot be interrupted by cancel(), and a tty inherited from the shell is +// left in blocking mode, so Go never registers fd 0 with the netpoller and +// SetReadDeadline returns ErrNoDeadline — polling is the only way to make the +// read abandonable. 100ms is imperceptible on teardown and costs ~10 idle +// wakeups/sec while attached, against the 4/sec the badge watcher already runs. +const AttachStdinPollInterval = 100 * time.Millisecond + +// AttachStdinReaderStopTimeout bounds how long cleanupAttach waits for the +// stdin reader to exit. Comfortably above AttachStdinPollInterval so a healthy +// reader always wins the race; the timeout only fires if the reader is wedged +// in a read that never returns, where proceeding beats hanging the TUI. +const AttachStdinReaderStopTimeout = time.Second + +// PollFdReady reports whether fd has input available within timeout. +// Retries on EINTR so a SIGWINCH mid-poll does not look like "no input". +// +// Exported because the SSH attach path (internal/session) needs the same +// abandonable-read primitive for the same reason — see QuiesceAttachInput. +func PollFdReady(fd int, timeout time.Duration) bool { + ms := int(timeout.Milliseconds()) + if ms < 1 { + ms = 1 + } + fds := []unix.PollFd{{Fd: int32(fd), Events: unix.POLLIN}} // #nosec G115 -- fd is a real OS file descriptor (small positive int), fits in int32 + for { + n, err := unix.Poll(fds, ms) + if err == unix.EINTR { + continue // retry after signal interruption + } + return err == nil && n > 0 + } +} + // IndexDetachKey returns the index of a control-key sequence in data, or -1 if // not found. detachByte is the raw ASCII byte (e.g. 0x11 for Ctrl+Q). // Handles three encodings: @@ -213,6 +249,127 @@ func resolveAttachInterrupt(chunk []byte, detach byte, opts AttachOptions) (int, return interruptIdx, outcome } +// attachStdinPump forwards host stdin into the attached tmux PTY, filtering +// terminal control replies and intercepting the detach / switch / scrollback +// interrupt keys. Extracted from AttachWithOptions so the loop's cancellation +// behaviour is unit-testable against an os.Pipe instead of a live tty. +type attachStdinPump struct { + in *os.File // host stdin, in raw mode + out io.Writer // the tmux attach PTY + detach byte + opts AttachOptions + startTime time.Time // attach start, for the reply-quarantine window + ioErrors chan<- error // buffered; sends are best-effort +} + +// run pumps input until ctx is cancelled, stdin reaches EOF, an interrupt key +// is pressed, or an I/O error occurs. It reports the switch intent and whether +// an interrupt key was what stopped it. +// +// Cancellation is checked between polls rather than relying on the read +// unblocking: os.Stdin.Read on a blocking tty is not interruptible, so a pump +// that ignored ctx would survive the attach and steal the caller's next +// keystroke. run therefore returns within roughly AttachStdinPollInterval of +// cancellation. +func (p *attachStdinPump) run(ctx context.Context) (SwitchIntent, bool) { + buf := make([]byte, 32) + var replyFilter termreply.Filter + fd := int(p.in.Fd()) // #nosec G115 -- an OS file descriptor is a small positive int + + reportErr := func(err error) { + select { + case p.ioErrors <- err: + default: // channel full, error already reported + } + } + + for { + if ctx.Err() != nil { + return SwitchNone, false + } + // Poll first so the read below never blocks longer than the interval. + if !PollFdReady(fd, AttachStdinPollInterval) { + continue + } + + n, err := p.in.Read(buf) + if err != nil { + if err == io.EOF { + return SwitchNone, false + } + reportErr(fmt.Errorf("stdin read error: %w", err)) + return SwitchNone, false + } + + chunk := buf[:n] + // Always run the reply filter: escape-string replies (DCS/OSC/etc.) + // can arrive long after the initial quarantine (e.g. iTerm2 + // XTVERSION reply on window focus/resize — #731). `armed` stays + // gated to the quarantine window so generic CSI pass-through + // works for keyboard input outside it. + armed := time.Since(p.startTime) < attachReplyQuarantine + chunk = replyFilter.Consume(chunk, armed, false) + if len(chunk) == 0 { + continue + } + + // Check for the detach key and any session-switch keys anywhere in + // the input chunk. Some terminals coalesce reads, so these must not + // require a single-byte read. Handles raw byte, xterm + // modifyOtherKeys, and kitty CSI u encodings. + // Whichever interrupt key appears first in the buffer wins, with + // detach > switch > scrollback precedence on a tie. Resolved by a + // pure helper so the precedence is unit-testable. + interruptIdx, outcome := resolveAttachInterrupt(chunk, p.detach, p.opts) + + if interruptIdx >= 0 { + // Forward any bytes before the interrupt key, then stop. + if interruptIdx > 0 { + if _, err := p.out.Write(chunk[:interruptIdx]); err != nil { + reportErr(fmt.Errorf("PTY write error: %w", err)) + return SwitchNone, false + } + } + return outcome, true + } + + // Forward other input to tmux PTY + if _, err := p.out.Write(chunk); err != nil { + reportErr(fmt.Errorf("PTY write error: %w", err)) + return SwitchNone, false + } + } +} + +// QuiesceAttachInput ends the attach's ownership of stdin: it waits for the +// stdin reader to exit, then drops whatever is sitting in the terminal's input +// queue and arms the reply quarantine. +// +// The order is the point, and both halves are load-bearing: +// +// - Wait first. Returning to Bubble Tea with the reader still parked on stdin +// leaves two readers on one tty; the stale one wins the next keystroke and +// the user loses a keypress. See attachStdinPump.run. +// - Flush second, on every exit path. Terminals emit capability/color replies +// as the session tears down, and those bytes would otherwise surface in the +// TUI as literal fragments. Flushing before the reader has stopped would +// just let it consume the post-flush bytes instead. +// +// readerDone is the reader's completion channel, timeout the backstop for a +// wedged reader, and flush/quarantine are injected so the ordering is testable +// without a live tty. It reports whether the reader exited on its own. +func QuiesceAttachInput(readerDone <-chan struct{}, timeout time.Duration, flush func(), quarantine func()) bool { + exited := false + select { + case <-readerDone: + exited = true + case <-time.After(timeout): + } + flush() + quarantine() + return exited +} + func waitForAttachOutputDrain(outputDone <-chan struct{}, timeout time.Duration) (bool, time.Duration) { start := time.Now() timer := time.NewTimer(timeout) @@ -404,7 +561,18 @@ func (s *Session) AttachWithOptions(ctx context.Context, opts AttachOptions) (Sw // Don't close sigwinch - signal.Stop() handles cleanup }() - // WaitGroup to track ALL goroutines (including SIGWINCH handler) + // WaitGroup to track ALL goroutines (including SIGWINCH handler). + // + // Deliberately never Wait()ed on, and it cannot be: the SIGWINCH handler + // only exits when the deferred close(sigwinchDone) runs, which is AFTER + // cleanupAttach, and the cmd.Wait() goroutine can outlive a detach by + // however long the tmux client takes to go away. Waiting here would + // deadlock the first and stall the second. + // + // The join that actually matters for correctness is stdinReaderDone, which + // cleanupAttach waits on individually — that is the goroutine that must not + // outlive the attach (see QuiesceAttachInput). The WaitGroup stays as + // documentation of goroutine ownership. var wg sync.WaitGroup // SIGWINCH handler goroutine - properly tracked in WaitGroup @@ -461,74 +629,35 @@ func (s *Session) AttachWithOptions(ctx context.Context, opts AttachOptions) (Sw } }() - // Goroutine 2: Read stdin, intercept detach key, forward rest to PTY + // Goroutine 2: Read stdin, intercept detach key, forward rest to PTY. + // + // stdinReaderDone closes when the pump returns. cleanupAttach waits on it + // so the attach never hands stdin back to Bubble Tea while this reader is + // still parked in a read. Two readers blocked on the same tty are woken in + // FIFO order, so a surviving reader wins the next keystroke, forwards it to + // the closed PTY and dies — eating exactly one keypress on the deck. + stdinReaderDone := make(chan struct{}) + pump := &attachStdinPump{ + in: os.Stdin, + out: ptmx, + detach: detach, + opts: opts, + startTime: startTime, + ioErrors: ioErrors, + } wg.Add(1) go func() { defer wg.Done() - buf := make([]byte, 32) - var replyFilter termreply.Filter - for { - n, err := os.Stdin.Read(buf) - if err != nil { - if err == io.EOF { - break - } - // Report stdin read error - select { - case ioErrors <- fmt.Errorf("stdin read error: %w", err): - default: - } - return - } - - chunk := buf[:n] - // Always run the reply filter: escape-string replies (DCS/OSC/etc.) - // can arrive long after the initial quarantine (e.g. iTerm2 - // XTVERSION reply on window focus/resize — #731). `armed` stays - // gated to the quarantine window so generic CSI pass-through - // works for keyboard input outside it. - armed := time.Since(startTime) < attachReplyQuarantine - chunk = replyFilter.Consume(chunk, armed, false) - if len(chunk) == 0 { - continue - } - - // Check for the detach key and any session-switch keys anywhere in - // the input chunk. Some terminals coalesce reads, so these must not - // require a single-byte read. Handles raw byte, xterm - // modifyOtherKeys, and kitty CSI u encodings. - // Whichever interrupt key appears first in the buffer wins, with - // detach > switch > scrollback precedence on a tie. Resolved by a - // pure helper so the precedence is unit-testable. - interruptIdx, outcome := resolveAttachInterrupt(chunk, detach, opts) - - if interruptIdx >= 0 { - // Forward any bytes before the interrupt key, then stop. - if interruptIdx > 0 { - if _, err := ptmx.Write(chunk[:interruptIdx]); err != nil { - select { - case ioErrors <- fmt.Errorf("PTY write error: %w", err): - default: - } - return - } - } - switchOutcome = outcome - close(detachCh) - cancel() - return - } - - // Forward other input to tmux PTY - if _, err := ptmx.Write(chunk); err != nil { - // Report PTY write error - select { - case ioErrors <- fmt.Errorf("PTY write error: %w", err): - default: - } - return - } + defer close(stdinReaderDone) + outcome, interrupted := pump.run(ctx) + if !interrupted { + return } + // Write switchOutcome before closing detachCh: the close establishes + // the happens-before edge the main goroutine relies on after <-detachCh. + switchOutcome = outcome + close(detachCh) + cancel() }() // Wait for command to finish - tracked in WaitGroup @@ -539,8 +668,6 @@ func (s *Session) AttachWithOptions(ctx context.Context, opts AttachOptions) (Sw cmdDone <- cmd.Wait() }() - didDetach := false - // Ensures we don't return to Bubble Tea while PTY output is still being written. // This avoids terminal style leakage (for example underline/hyperlink state) // from the attached client into the Agent Deck UI. @@ -552,14 +679,16 @@ func (s *Session) AttachWithOptions(ctx context.Context, opts AttachOptions) (Sw cancel() _ = ptmx.Close() _, _ = waitForAttachOutputDrain(outputDone, attachOutputDrainTimeout) - // Prompts can issue terminal capability/color queries as they redraw during - // detach. Kitty replies on stdin; if those queued bytes survive until Bubble Tea - // resumes, they can leak as literal fragments like terminal version strings or - // rgb payloads in the TUI. - if didDetach { - _ = flushDetachInput(int(os.Stdin.Fd())) - termreply.QuarantineFor(attachReplyQuarantine) - } + // Hand stdin back: stop the reader, then flush the input queue and arm + // the quarantine. cancel() above makes the reader exit within one poll + // interval; the timeout is a wedged-reader backstop, not the expected + // path. See QuiesceAttachInput for why the order matters. + QuiesceAttachInput( + stdinReaderDone, + AttachStdinReaderStopTimeout, + func() { _ = FlushInput(int(os.Stdin.Fd())) }, + func() { termreply.QuarantineFor(attachReplyQuarantine) }, + ) // Clear host terminal scrollback before returning to TUI. // The on-attach clear at the top of Attach() covers the "next attach" direction; // this covers the "on detach" direction for belt-and-suspenders coverage @@ -579,7 +708,6 @@ func (s *Session) AttachWithOptions(ctx context.Context, opts AttachOptions) (Sw select { case <-detachCh: // User pressed the detach key, detach gracefully - didDetach = true attachErr = nil case err := <-cmdDone: if err != nil { diff --git a/internal/tmux/pty_flush_linux.go b/internal/tmux/pty_flush_linux.go index 3948070a..372cc8da 100644 --- a/internal/tmux/pty_flush_linux.go +++ b/internal/tmux/pty_flush_linux.go @@ -5,10 +5,10 @@ package tmux import "golang.org/x/sys/unix" -// flushDetachInput drains pending input from the terminal fd so that any +// FlushInput drains pending input from the terminal fd so that any // bytes typed after the detach key (e.g. Ctrl+Q) do not bleed into the // TUI's stdin after the attach returns. Linux implementation uses // TCFLSH; darwin/bsd live in pty_flush_other.go. -func flushDetachInput(fd int) error { +func FlushInput(fd int) error { return unix.IoctlSetInt(fd, unix.TCFLSH, unix.TCIFLUSH) } diff --git a/internal/tmux/pty_flush_other.go b/internal/tmux/pty_flush_other.go index c1bd04c0..1673482b 100644 --- a/internal/tmux/pty_flush_other.go +++ b/internal/tmux/pty_flush_other.go @@ -5,10 +5,10 @@ package tmux import "golang.org/x/sys/unix" -// flushDetachInput on darwin/bsd uses TIOCFLUSH with the FREAD direction +// FlushInput on darwin/bsd uses TIOCFLUSH with the FREAD direction // bit. Equivalent to Linux's TCIFLUSH on TCFLSH — drains pending input // from the terminal fd after detach. -func flushDetachInput(fd int) error { +func FlushInput(fd int) error { // FREAD == 1; flushing the read queue matches TCIFLUSH semantics. return unix.IoctlSetInt(fd, unix.TIOCFLUSH, 1) }