diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d0bd243..b5fa753 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,10 +40,41 @@ jobs: components: clippy - run: cargo clippy --all-targets --all-features -- -D warnings - test: - name: Test - runs-on: ubuntu-latest + test-matrix: + strategy: + matrix: + os: [ubuntu-latest, macos-latest] + runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@stable - run: cargo test --all-targets + + test: + name: Test + runs-on: ubuntu-latest + needs: test-matrix + if: always() + steps: + - run: | + if [[ "${{ needs.test-matrix.result }}" != "success" ]]; then + exit 1 + fi + + integration: + name: Integration Tests + strategy: + matrix: + os: [ubuntu-latest, macos-latest] + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + - name: Install expect (Linux) + if: runner.os == 'Linux' + run: sudo apt-get update && sudo apt-get install -y expect + - name: Install expect (macOS) + if: runner.os == 'macOS' + run: command -v expect || brew install expect + - run: cargo build + - run: expect tests/integration.exp diff --git a/crates/claude-chill/src/bin.rs b/crates/claude-chill/src/bin.rs index 5a056c3..446d37c 100644 --- a/crates/claude-chill/src/bin.rs +++ b/crates/claude-chill/src/bin.rs @@ -45,7 +45,7 @@ fn main() -> ExitCode { } }, Err(e) => { - eprintln!("Failed to start proxy: {}", e); + eprintln!("Failed to start proxy: {:#}", e); ExitCode::from(1) } } diff --git a/crates/claude-chill/src/proxy.rs b/crates/claude-chill/src/proxy.rs index c1736e1..de9a4a4 100644 --- a/crates/claude-chill/src/proxy.rs +++ b/crates/claude-chill/src/proxy.rs @@ -6,12 +6,15 @@ use crate::escape_sequences::{ use crate::line_buffer::LineBuffer; use anyhow::{Context, Result}; use memchr::memmem; +use nix::errno::Errno; use nix::fcntl::{FcntlArg, OFlag, fcntl}; +use nix::poll::{PollFd, PollFlags, PollTimeout, poll}; use nix::pty::{Winsize, openpty}; -use nix::sys::signal::{SaFlags, SigAction, SigHandler, SigSet, Signal, sigaction}; +use nix::sys::signal::{SaFlags, SigAction, SigHandler, SigSet, Signal, kill, sigaction}; use nix::sys::termios::{SetArg, Termios, cfmakeraw, tcgetattr, tcsetattr}; +use nix::unistd::{Pid, isatty, read, write}; use std::io; -use std::os::fd::{AsFd, AsRawFd, OwnedFd}; +use std::os::fd::{AsFd, AsRawFd, BorrowedFd, OwnedFd}; use std::os::unix::process::CommandExt; use std::process::{Child, Command, ExitStatus}; use std::sync::atomic::{AtomicBool, Ordering}; @@ -50,6 +53,29 @@ impl Default for ProxyConfig { } } +struct TerminalGuard { + original_termios: Option, +} + +impl TerminalGuard { + fn new() -> Result { + let original_termios = setup_raw_mode()?; + Ok(Self { original_termios }) + } + + fn take(mut self) -> Option { + self.original_termios.take() + } +} + +impl Drop for TerminalGuard { + fn drop(&mut self) { + if let Some(ref termios) = self.original_termios { + let _ = tcsetattr(io::stdin(), SetArg::TCSANOW, termios); + } + } +} + pub struct Proxy { config: ProxyConfig, pty_master: OwnedFd, @@ -78,7 +104,7 @@ impl Proxy { let winsize = get_terminal_size()?; let pty = openpty(&winsize, None).context("openpty failed")?; - let original_termios = setup_raw_mode()?; + let terminal_guard = TerminalGuard::new()?; setup_signal_handlers()?; let slave_fd = pty.slave.as_raw_fd(); @@ -90,7 +116,7 @@ impl Proxy { if libc::setsid() == -1 { return Err(io::Error::last_os_error()); } - if libc::ioctl(slave_fd, libc::TIOCSCTTY, 0) == -1 { + if libc::ioctl(slave_fd, libc::TIOCSCTTY as libc::c_ulong, 0) == -1 { return Err(io::Error::last_os_error()); } if libc::dup2(slave_fd, 0) == -1 { @@ -119,7 +145,7 @@ impl Proxy { config, pty_master: pty.master, child, - original_termios, + original_termios: terminal_guard.take(), sync_buffer: Vec::with_capacity(SYNC_BUFFER_CAPACITY), in_sync_block: false, in_lookback_mode: false, @@ -139,9 +165,8 @@ impl Proxy { } pub fn run(&mut self) -> Result { - let stdin_raw = io::stdin().as_raw_fd(); - let stdout_raw = io::stdout().as_raw_fd(); - let master_raw = self.pty_master.as_raw_fd(); + let stdin_fd = io::stdin(); + let stdout_fd = io::stdout(); let mut buf = [0u8; 65536]; @@ -156,62 +181,56 @@ impl Proxy { self.forward_signal(Signal::SIGTERM); } + let master_fd = unsafe { BorrowedFd::borrow_raw(self.pty_master.as_raw_fd()) }; + let stdin_borrowed = unsafe { BorrowedFd::borrow_raw(stdin_fd.as_raw_fd()) }; + let mut poll_fds = [ - libc::pollfd { - fd: master_raw, - events: libc::POLLIN, - revents: 0, - }, - libc::pollfd { - fd: stdin_raw, - events: libc::POLLIN, - revents: 0, - }, + PollFd::new(master_fd, PollFlags::POLLIN), + PollFd::new(stdin_borrowed, PollFlags::POLLIN), ]; - let poll_ret = unsafe { libc::poll(poll_fds.as_mut_ptr(), 2, 100) }; - if poll_ret == 0 { - continue; - } - if poll_ret < 0 { - let err = unsafe { *libc::__errno_location() }; - if err == libc::EINTR { - continue; - } - anyhow::bail!("poll failed: errno {}", err); + match poll(&mut poll_fds, PollTimeout::from(100u16)) { + Ok(0) => continue, + Ok(_) => {} + Err(Errno::EINTR) => continue, + Err(e) => anyhow::bail!("poll failed: {}", e), } - if poll_fds[0].revents & libc::POLLIN != 0 { - match libc_read(master_raw, &mut buf) { - Ok(0) => break, - Ok(n) => self.process_output(&buf[..n], stdout_raw)?, - Err(e) if e == libc::EAGAIN => {} - Err(e) if e == libc::EIO => break, - Err(e) => anyhow::bail!("read from pty failed: errno {}", e), + if let Some(revents) = poll_fds[0].revents() { + if revents.contains(PollFlags::POLLIN) { + match nix_read(&self.pty_master, &mut buf) { + Ok(0) => break, + Ok(n) => self.process_output(&buf[..n], &stdout_fd)?, + Err(Errno::EAGAIN) => {} + Err(Errno::EIO) => break, + Err(e) => anyhow::bail!("read from pty failed: {}", e), + } + } + if revents.contains(PollFlags::POLLHUP) { + break; } } - if poll_fds[0].revents & libc::POLLHUP != 0 { - break; - } - if poll_fds[1].revents & libc::POLLIN != 0 { - match libc_read(stdin_raw, &mut buf) { + if let Some(revents) = poll_fds[1].revents() + && revents.contains(PollFlags::POLLIN) + { + match nix_read(&stdin_fd, &mut buf) { Ok(0) => break, - Ok(n) => self.process_input(&buf[..n], stdout_raw)?, - Err(e) if e == libc::EAGAIN => {} - Err(e) => anyhow::bail!("read from stdin failed: errno {}", e), + Ok(n) => self.process_input(&buf[..n], &stdout_fd)?, + Err(Errno::EAGAIN) => {} + Err(e) => anyhow::bail!("read from stdin failed: {}", e), } } } if !self.sync_buffer.is_empty() { - write_all_raw(stdout_raw, &self.sync_buffer)?; + write_all(&stdout_fd, &self.sync_buffer)?; } self.wait_child() } - fn process_output(&mut self, data: &[u8], stdout_fd: i32) -> Result<()> { + fn process_output(&mut self, data: &[u8], stdout_fd: &F) -> Result<()> { if self.in_alternate_screen { return self.process_output_alt_screen(data, stdout_fd); } @@ -228,15 +247,15 @@ impl Proxy { if self.in_sync_block { self.sync_buffer .extend_from_slice(&data[pos..pos + alt_pos]); - write_all_raw(stdout_fd, &self.sync_buffer)?; + write_all(stdout_fd, &self.sync_buffer)?; self.sync_buffer.clear(); self.in_sync_block = false; } else if alt_pos > 0 { - write_all_raw(stdout_fd, &data[pos..pos + alt_pos])?; + write_all(stdout_fd, &data[pos..pos + alt_pos])?; } self.in_alternate_screen = true; let seq_len = self.alt_screen_enter_len(&data[pos + alt_pos..]); - write_all_raw(stdout_fd, &data[pos + alt_pos..pos + alt_pos + seq_len])?; + write_all(stdout_fd, &data[pos + alt_pos..pos + alt_pos + seq_len])?; return self.process_output_alt_screen(&data[pos + alt_pos + seq_len..], stdout_fd); } @@ -253,14 +272,14 @@ impl Proxy { } } else if let Some(idx) = self.sync_start_finder.find(&data[pos..]) { if idx > 0 { - write_all_raw(stdout_fd, &data[pos..pos + idx])?; + write_all(stdout_fd, &data[pos..pos + idx])?; } self.in_sync_block = true; self.sync_buffer.clear(); self.sync_buffer.extend_from_slice(SYNC_START); pos += idx + SYNC_START.len(); } else { - write_all_raw(stdout_fd, &data[pos..])?; + write_all(stdout_fd, &data[pos..])?; break; } } @@ -268,15 +287,15 @@ impl Proxy { Ok(()) } - fn process_output_alt_screen(&mut self, data: &[u8], stdout_fd: i32) -> Result<()> { + fn process_output_alt_screen(&mut self, data: &[u8], stdout_fd: &F) -> Result<()> { if let Some(exit_pos) = self.find_alt_screen_exit(data) { - write_all_raw(stdout_fd, &data[..exit_pos])?; + write_all(stdout_fd, &data[..exit_pos])?; let seq_len = self.alt_screen_exit_len(&data[exit_pos..]); - write_all_raw(stdout_fd, &data[exit_pos..exit_pos + seq_len])?; + write_all(stdout_fd, &data[exit_pos..exit_pos + seq_len])?; self.in_alternate_screen = false; return self.process_output(&data[exit_pos + seq_len..], stdout_fd); } - write_all_raw(stdout_fd, data) + write_all(stdout_fd, data) } fn find_alt_screen_enter(&self, data: &[u8]) -> Option { @@ -317,7 +336,7 @@ impl Proxy { } } - fn flush_sync_block(&mut self, stdout_fd: i32) -> Result<()> { + fn flush_sync_block(&mut self, stdout_fd: &F) -> Result<()> { let has_clear_screen = self.clear_screen_finder.find(&self.sync_buffer).is_some(); let has_cursor_home = self.cursor_home_finder.find(&self.sync_buffer).is_some(); let is_full_redraw = has_clear_screen && has_cursor_home; @@ -329,9 +348,9 @@ impl Proxy { if is_full_redraw { self.create_truncated_output(); - write_all_raw(stdout_fd, &self.output_buffer)?; + write_all(stdout_fd, &self.output_buffer)?; } else { - write_all_raw(stdout_fd, &self.sync_buffer)?; + write_all(stdout_fd, &self.sync_buffer)?; } Ok(()) @@ -348,11 +367,9 @@ impl Proxy { self.output_buffer.extend_from_slice(SYNC_END); } - fn process_input(&mut self, data: &[u8], stdout_fd: i32) -> Result<()> { - let master_fd = self.pty_master.as_raw_fd(); - + fn process_input(&mut self, data: &[u8], stdout_fd: &F) -> Result<()> { if self.in_alternate_screen { - return write_all_raw(master_fd, data); + return write_all(&self.pty_master, data); } for &byte in data { @@ -367,7 +384,7 @@ impl Proxy { if self.input_buffer.len() > self.config.lookback_sequence.len() { let excess = self.input_buffer.len() - self.config.lookback_sequence.len(); if !self.in_lookback_mode { - write_all_raw(master_fd, &self.input_buffer[..excess])?; + write_all(&self.pty_master, &self.input_buffer[..excess])?; } self.input_buffer.drain(..excess); } @@ -385,7 +402,7 @@ impl Proxy { .starts_with(&self.input_buffer) { if !self.in_lookback_mode { - write_all_raw(master_fd, &self.input_buffer)?; + write_all(&self.pty_master, &self.input_buffer)?; } self.input_buffer.clear(); } @@ -400,21 +417,21 @@ impl Proxy { self.output_buffer.clear(); self.history.append_all(&mut self.output_buffer); - let stdout_fd = io::stdout().as_raw_fd(); - write_all_raw(stdout_fd, CLEAR_SCREEN)?; - write_all_raw(stdout_fd, CURSOR_HOME)?; - write_all_raw(stdout_fd, &self.output_buffer)?; + let stdout_fd = io::stdout(); + write_all(&stdout_fd, CLEAR_SCREEN)?; + write_all(&stdout_fd, CURSOR_HOME)?; + write_all(&stdout_fd, &self.output_buffer)?; let exit_msg = format!( "\r\n\x1b[7m--- LOOKBACK MODE: press {} or Ctrl+C to exit ---\x1b[0m\r\n", self.config.lookback_key ); - write_all_raw(stdout_fd, exit_msg.as_bytes())?; + write_all(&stdout_fd, exit_msg.as_bytes())?; Ok(()) } - fn exit_lookback_mode(&mut self, stdout_fd: i32) -> Result<()> { + fn exit_lookback_mode(&mut self, stdout_fd: &F) -> Result<()> { self.in_lookback_mode = false; let cached = std::mem::take(&mut self.lookback_cache); @@ -425,9 +442,9 @@ impl Proxy { self.output_buffer.clear(); self.history.append_all(&mut self.output_buffer); - write_all_raw(stdout_fd, CLEAR_SCREEN)?; - write_all_raw(stdout_fd, CURSOR_HOME)?; - write_all_raw(stdout_fd, &self.output_buffer)?; + write_all(stdout_fd, CLEAR_SCREEN)?; + write_all(stdout_fd, CURSOR_HOME)?; + write_all(stdout_fd, &self.output_buffer)?; Ok(()) } @@ -435,17 +452,19 @@ impl Proxy { fn forward_winsize(&self) -> Result<()> { if let Ok(winsize) = get_terminal_size() { unsafe { - libc::ioctl(self.pty_master.as_raw_fd(), libc::TIOCSWINSZ, &winsize); + libc::ioctl( + self.pty_master.as_raw_fd(), + libc::TIOCSWINSZ as libc::c_ulong, + &winsize, + ); } } Ok(()) } fn forward_signal(&self, signal: Signal) { - let pid = self.child.id(); - unsafe { - libc::kill(pid as i32, signal as i32); - } + let pid = Pid::from_raw(self.child.id() as i32); + let _ = kill(pid, signal); } fn wait_child(&mut self) -> Result { @@ -466,7 +485,13 @@ impl Drop for Proxy { fn get_terminal_size() -> Result { let mut ws: Winsize = unsafe { std::mem::zeroed() }; - let ret = unsafe { libc::ioctl(io::stdout().as_raw_fd(), libc::TIOCGWINSZ, &mut ws) }; + let ret = unsafe { + libc::ioctl( + io::stdout().as_raw_fd(), + libc::TIOCGWINSZ as libc::c_ulong, + &mut ws, + ) + }; if ret == -1 { ws.ws_row = 24; ws.ws_col = 80; @@ -487,8 +512,7 @@ fn exit_code_from_status(status: ExitStatus) -> i32 { fn setup_raw_mode() -> Result> { let stdin = io::stdin(); - let is_tty = unsafe { libc::isatty(stdin.as_raw_fd()) == 1 }; - if !is_tty { + if !isatty(&stdin).unwrap_or(false) { return Ok(None); } @@ -524,32 +548,18 @@ fn set_nonblocking(fd: &Fd) -> Result<()> { Ok(()) } -fn write_all_raw(fd: i32, data: &[u8]) -> Result<()> { +fn write_all(fd: &F, data: &[u8]) -> Result<()> { let mut written = 0; while written < data.len() { - match libc_write(fd, &data[written..]) { + match write(fd, &data[written..]) { Ok(n) => written += n, - Err(e) if e == libc::EAGAIN || e == libc::EINTR => continue, - Err(e) => anyhow::bail!("write failed: errno {}", e), + Err(Errno::EAGAIN) | Err(Errno::EINTR) => continue, + Err(e) => anyhow::bail!("write failed: {}", e), } } Ok(()) } -fn libc_read(fd: i32, buf: &mut [u8]) -> Result { - let ret = unsafe { libc::read(fd, buf.as_mut_ptr() as *mut libc::c_void, buf.len()) }; - if ret < 0 { - Err(unsafe { *libc::__errno_location() }) - } else { - Ok(ret as usize) - } -} - -fn libc_write(fd: i32, buf: &[u8]) -> Result { - let ret = unsafe { libc::write(fd, buf.as_ptr() as *const libc::c_void, buf.len()) }; - if ret < 0 { - Err(unsafe { *libc::__errno_location() }) - } else { - Ok(ret as usize) - } +fn nix_read(fd: &F, buf: &mut [u8]) -> Result { + read(fd.as_fd(), buf) } diff --git a/tests/integration.exp b/tests/integration.exp new file mode 100644 index 0000000..3a66cb9 --- /dev/null +++ b/tests/integration.exp @@ -0,0 +1,285 @@ +#!/usr/bin/expect -f +# +# Integration tests for claude-chill +# Tests lookback mode and alt-screen handling +# +# Usage: expect tests/integration.exp [path-to-binary] +# + +set timeout 10 +set binary [lindex $argv 0] +if {$binary eq ""} { + set binary "./target/debug/claude-chill" +} + +# Use a simple, predictable prompt +set env(PS1) "TEST> " +set prompt "TEST> " + +# Ensure TERM is set for proper terminal emulation (needed for less/vim alt screen) +set env(TERM) "xterm-256color" + +# Suppress macOS bash zsh migration warning +set env(BASH_SILENCE_DEPRECATION_WARNING) "1" + +proc fail {msg} { + puts "\n\033\[31mFAIL: $msg\033\[0m" + exit 1 +} + +proc pass {msg} { + puts "\033\[32mPASS: $msg\033\[0m" +} + +# Build first if binary doesn't exist +if {![file exists $binary]} { + puts "Building..." + if {[catch {exec cargo build 2>@1}]} { + fail "cargo build failed" + } +} + +puts "Using binary: $binary" +puts "" + +############################################# +# Test 1: Basic spawn and exit +############################################# +puts "Test 1: Basic spawn and exit" + +spawn $binary -- /bin/bash --norc --noprofile +expect { + $prompt { } + timeout { fail "shell prompt not received" } +} + +send "echo hello\r" +expect { + "hello" { } + timeout { fail "echo output not received" } +} + +send "exit\r" +expect { + eof { pass "Basic spawn and exit" } + timeout { fail "did not exit cleanly" } +} + +############################################# +# Test 2: Lookback mode toggle +############################################# +puts "\nTest 2: Lookback mode toggle" + +spawn $binary -- /bin/bash --norc --noprofile +expect $prompt + +# Generate some output to look back at +send "echo line1\r" +expect $prompt +send "echo line2\r" +expect $prompt +send "echo line3\r" +expect $prompt + +# Enter lookback mode (Ctrl+^ = 0x1E) +send "\x1e" +expect { + "LOOKBACK MODE" { } + timeout { fail "lookback mode not entered" } +} + +# Exit lookback mode (same key again) +sleep 0.3 +send "\x1e" +sleep 0.5 + +# Should be back to normal - verify shell still works +send "echo after_lookback\r" +expect { + "after_lookback" { } + timeout { fail "shell not responsive after lookback" } +} + +send "exit\r" +expect { + eof { pass "Lookback mode toggle" } + timeout { fail "did not exit cleanly" } +} + +############################################# +# Test 3: Lookback mode exit with Ctrl+C +############################################# +puts "\nTest 3: Lookback mode exit with Ctrl+C" + +spawn $binary -- /bin/bash --norc --noprofile +expect $prompt + +send "echo test_output\r" +expect $prompt + +# Enter lookback mode +send "\x1e" +expect "LOOKBACK MODE" + +# Exit with Ctrl+C instead +send "\x03" + +# Should be back to normal +sleep 0.5 +send "echo still_works\r" +expect { + "still_works" { } + timeout { fail "shell not responsive after Ctrl+C exit" } +} + +send "exit\r" +expect { + eof { pass "Lookback mode exit with Ctrl+C" } + timeout { fail "did not exit cleanly" } +} + +############################################# +# Test 4: Alt screen mode (using less) +############################################# +puts "\nTest 4: Alt screen mode" + +# Check if less is available +if {[catch {exec which less}]} { + puts "SKIP: less not found" +} else { + spawn $binary -- /bin/bash --norc --noprofile + expect $prompt + + # Create a temp file and view with less + send "echo -e 'line1\\nline2\\nline3' | less\r" + + # Wait for less to start (alt screen) + sleep 0.5 + + # Try lookback key - should pass through to less, not trigger lookback + # (In alt screen, lookback is disabled) + send "\x1e" + sleep 0.3 + + # Exit less + send "q" + expect $prompt + + # Now test that lookback works again after exiting alt screen + send "echo back_from_less\r" + expect $prompt + + send "\x1e" + expect { + "LOOKBACK MODE" { } + timeout { fail "lookback mode not working after alt screen" } + } + + send "\x1e" + sleep 0.3 + + send "exit\r" + expect { + eof { pass "Alt screen mode" } + timeout { fail "did not exit cleanly" } + } +} + +############################################# +# Test 5: Alt screen with vi/vim (if available) +############################################# +puts "\nTest 5: Alt screen with vi/vim" + +# Try vim first, then vi +set vi_cmd "" +if {![catch {exec which vim}]} { + set vi_cmd "vim" +} elseif {![catch {exec which vi}]} { + set vi_cmd "vi" +} + +if {$vi_cmd eq ""} { + puts "SKIP: vi/vim not found" +} else { + spawn $binary -- /bin/bash --norc --noprofile + expect $prompt + + # Start vi/vim with no config to avoid user rc issues + send "$vi_cmd -u NONE\r" + sleep 1 + + # In vi, Ctrl+^ switches between buffers - should NOT trigger lookback + send "\x1e" + sleep 0.3 + + # Exit vi + send ":q!\r" + expect $prompt + + # Lookback should work now + send "\x1e" + expect { + "LOOKBACK MODE" { } + timeout { fail "lookback mode not working after $vi_cmd" } + } + + send "\x1e" + sleep 0.3 + + send "exit\r" + expect { + eof { pass "Alt screen with $vi_cmd" } + timeout { fail "did not exit cleanly" } + } +} + +############################################# +# Test 6: Dumb terminal mode (no alt screen) +############################################# +puts "\nTest 6: Dumb terminal mode" + +# Temporarily set TERM to dumb +set env(TERM) "dumb" + +spawn $binary -- /bin/bash --norc --noprofile +expect $prompt + +# Generate some output +send "echo dumb_test\r" +expect $prompt + +# Lookback should work since there's no alt screen +send "\x1e" +expect { + "LOOKBACK MODE" { } + timeout { fail "lookback mode not working in dumb terminal" } +} + +# Exit lookback +send "\x1e" +sleep 0.3 + +# Shell should still work +send "echo after_dumb_lookback\r" +expect { + "after_dumb_lookback" { } + timeout { fail "shell not responsive after lookback in dumb mode" } +} + +send "exit\r" +expect { + eof { pass "Dumb terminal mode" } + timeout { fail "did not exit cleanly" } +} + +# Restore TERM for any subsequent tests +set env(TERM) "xterm-256color" + +############################################# +# Summary +############################################# +puts "\n\033\[32m========================================\033\[0m" +puts "\033\[32mAll tests passed!\033\[0m" +puts "\033\[32m========================================\033\[0m" + +exit 0