From 988d54bcd1e6ccdd721e6422bc9085fcac315263 Mon Sep 17 00:00:00 2001 From: Rick Morgans Date: Sat, 14 Mar 2026 22:48:16 +1030 Subject: [PATCH 01/22] fix: close leaked fds in openpty fallback, add static assertions - openpty fallback now closes master fd on grantpt/unlockpt/ptsname failure instead of leaking it - Add _Static_assert for SCROLLBACK_SIZE power-of-two requirement and packet buffer fitting in uint8_t length field Co-Authored-By: Claude Opus 4.6 (1M context) --- atch.h | 2 ++ config.h | 2 ++ master.c | 21 ++++++++++++++------- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/atch.h b/atch.h index 0b59f48..e096256 100644 --- a/atch.h +++ b/atch.h @@ -115,6 +115,8 @@ struct packet struct winsize ws; } u; }; +_Static_assert(sizeof(((struct packet *)0)->u.buf) <= 255, + "packet buffer must fit in uint8_t length"); /* ** The master sends a simple stream of text to the attaching clients, without diff --git a/config.h b/config.h index 398c905..3b70f94 100644 --- a/config.h +++ b/config.h @@ -22,6 +22,8 @@ #ifndef SCROLLBACK_SIZE #define SCROLLBACK_SIZE (128 * 1024) #endif +_Static_assert((SCROLLBACK_SIZE & (SCROLLBACK_SIZE - 1)) == 0, + "SCROLLBACK_SIZE must be a power of two"); /* Maximum size of the on-disk session log; older bytes are trimmed on open */ #ifndef LOG_MAX_SIZE diff --git a/master.c b/master.c index 16f29e3..164196e 100644 --- a/master.c +++ b/master.c @@ -802,29 +802,29 @@ int openpty(int *amaster, int *aslave, char *name, struct termios *termp, struct winsize *winp) { - int master, slave; + int master = -1, slave = -1; char *buf; master = open("/dev/ptmx", O_RDWR); if (master < 0) return -1; if (grantpt(master) < 0) - return -1; + goto fail; if (unlockpt(master) < 0) - return -1; + goto fail; buf = ptsname(master); if (!buf) - return -1; + goto fail; slave = open(buf, O_RDWR | O_NOCTTY); if (slave < 0) - return -1; + goto fail; #ifdef I_PUSH if (ioctl(slave, I_PUSH, "ptem") < 0) - return -1; + goto fail; if (ioctl(slave, I_PUSH, "ldterm") < 0) - return -1; + goto fail; #endif *amaster = master; @@ -836,6 +836,13 @@ openpty(int *amaster, int *aslave, char *name, struct termios *termp, if (winp) ioctl(slave, TIOCSWINSZ, winp); return 0; + +fail: + if (master >= 0) + close(master); + if (slave >= 0) + close(slave); + return -1; } pid_t From 5dbb1f6b5da941aee8dfde343f002c34d0efac05 Mon Sep 17 00:00:00 2001 From: Rick Morgans Date: Sat, 14 Mar 2026 22:49:55 +1030 Subject: [PATCH 02/22] fix: defensive checks for cwd restore, malloc, and log writes - socket_with_chdir: always restore cwd, even on socket failure - expand_sockname: check malloc return before use - rotate_log/pty_activity: disable logging on write failure instead of silently losing data Co-Authored-By: Claude Opus 4.6 (1M context) --- atch.c | 18 ++++++++++++++---- master.c | 10 ++++++++-- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/atch.c b/atch.c index ff8f2f1..c0a7c2b 100644 --- a/atch.c +++ b/atch.c @@ -76,11 +76,17 @@ int socket_with_chdir(char *path, int (*fn)(char *)) *slash = '\0'; s = chdir(path) >= 0 ? fn(slash + 1) : -1; *slash = '/'; - if (s >= 0 && fchdir(dirfd) < 0) { - close(s); - s = -1; + /* Always restore cwd, regardless of socket operation result */ + { + int saved_errno = errno; + if (fchdir(dirfd) < 0) { + if (s >= 0) close(s); + close(dirfd); + return -1; + } + close(dirfd); + errno = saved_errno; } - close(dirfd); return s; } @@ -275,6 +281,10 @@ static void expand_sockname(void) mkdir(dir, 0700); fulllen = strlen(dir) + 1 + strlen(sockname); full = malloc(fulllen + 1); + if (!full) { + printf("%s: out of memory\n", progname); + exit(1); + } snprintf(full, fulllen + 1, "%s/%s", dir, sockname); sockname = full; } diff --git a/master.c b/master.c index 16f29e3..0afaa09 100644 --- a/master.c +++ b/master.c @@ -67,7 +67,10 @@ static void rotate_log(void) if (n > 0) { ftruncate(log_fd, 0); lseek(log_fd, 0, SEEK_SET); - write(log_fd, buf, (size_t)n); + if (write(log_fd, buf, (size_t)n) < 0) { + close(log_fd); + log_fd = -1; + } } free(buf); } @@ -388,7 +391,10 @@ static void pty_activity(int s) } scrollback_append(buf, (size_t)len); if (log_fd >= 0) { - write(log_fd, buf, (size_t)len); + if (write(log_fd, buf, (size_t)len) < 0) { + close(log_fd); + log_fd = -1; + } log_written += (size_t)len; if (log_written >= log_max_size) { rotate_log(); From c6a9d1846adfb2f01ee8d7d53119e562ea01b988 Mon Sep 17 00:00:00 2001 From: Rick Morgans Date: Sat, 14 Mar 2026 22:50:34 +1030 Subject: [PATCH 03/22] fix: retry partial writes in packet and push paths Extract write_all() and use it everywhere a socket write must be complete-or-fail. Fixes spurious disconnects under memory pressure or signal interruption that returns a short count. Co-Authored-By: Claude Opus 4.6 (1M context) --- attach.c | 79 +++++++++++++++++++++++++++----------------------------- 1 file changed, 38 insertions(+), 41 deletions(-) diff --git a/attach.c b/attach.c index 2de74ac..8630e8d 100644 --- a/attach.c +++ b/attach.c @@ -27,8 +27,9 @@ char const *clear_csi_data(void) return "\033[999H\r\n"; } -/* Write buf to fd handling partial writes. Exit on failure. */ -void write_buf_or_fail(int fd, const void *buf, size_t count) +/* Write all of buf to fd, retrying on short writes and EINTR. +** Returns 0 on success, -1 on failure (errno is set). */ +static int write_all(int fd, const void *buf, size_t count) { while (count != 0) { ssize_t ret = write(fd, buf, count); @@ -36,49 +37,49 @@ void write_buf_or_fail(int fd, const void *buf, size_t count) if (ret >= 0) { buf = (const char *)buf + ret; count -= ret; - } else if (ret < 0 && errno == EINTR) + } else if (errno == EINTR) continue; - else { - if (session_start) { - char age[32]; - session_age(age, sizeof(age)); - printf - ("%s[%s: session '%s' write failed after %s]\r\n", - clear_csi_data(), progname, - session_shortname(), age); - } else { - printf("%s[%s: write failed]\r\n", - clear_csi_data(), progname); - } - exit(1); + else + return -1; + } + return 0; +} + +/* Write buf to fd handling partial writes. Exit on failure. */ +void write_buf_or_fail(int fd, const void *buf, size_t count) +{ + if (write_all(fd, buf, count) < 0) { + if (session_start) { + char age[32]; + session_age(age, sizeof(age)); + printf + ("%s[%s: session '%s' write failed after %s]\r\n", + clear_csi_data(), progname, + session_shortname(), age); + } else { + printf("%s[%s: write failed]\r\n", + clear_csi_data(), progname); } + exit(1); } } /* Write pkt to fd. Exit on failure. */ void write_packet_or_fail(int fd, const struct packet *pkt) { - while (1) { - ssize_t ret = write(fd, pkt, sizeof(struct packet)); - - if (ret == sizeof(struct packet)) - return; - else if (ret < 0 && errno == EINTR) - continue; - else { - if (session_start) { - char age[32]; - session_age(age, sizeof(age)); - printf - ("%s[%s: session '%s' write failed after %s]\r\n", - clear_csi_data(), progname, - session_shortname(), age); - } else { - printf("%s[%s: write failed]\r\n", - clear_csi_data(), progname); - } - exit(1); + if (write_all(fd, pkt, sizeof(struct packet)) < 0) { + if (session_start) { + char age[32]; + session_age(age, sizeof(age)); + printf + ("%s[%s: session '%s' write failed after %s]\r\n", + clear_csi_data(), progname, + session_shortname(), age); + } else { + printf("%s[%s: write failed]\r\n", + clear_csi_data(), progname); } + exit(1); } } @@ -497,11 +498,7 @@ int push_main() } pkt.len = len; - len = write(s, &pkt, sizeof(struct packet)); - if (len != sizeof(struct packet)) { - if (len >= 0) - errno = EPIPE; - + if (write_all(s, &pkt, sizeof(struct packet)) < 0) { printf("%s: %s: %s\n", progname, sockname, strerror(errno)); return 1; From 851934a7678f4c6cada8f8a5c3adc8b93355a57e Mon Sep 17 00:00:00 2001 From: Rick Morgans Date: Sat, 14 Mar 2026 23:05:18 +1030 Subject: [PATCH 04/22] fix: use write_all in send_kill (missed in initial fix) send_kill() still used a bare write() for the kill packet. Apply the same write_all() retry loop as push and attach paths. Co-Authored-By: Claude Opus 4.6 (1M context) --- attach.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/attach.c b/attach.c index 8630e8d..f49ac9a 100644 --- a/attach.c +++ b/attach.c @@ -518,9 +518,9 @@ static int send_kill(int sig) memset(&pkt, 0, sizeof(pkt)); pkt.type = MSG_KILL; pkt.len = (unsigned char)sig; - ret = write(s, &pkt, sizeof(pkt)); + ret = write_all(s, &pkt, sizeof(pkt)); close(s); - return (ret == sizeof(pkt)) ? 0 : -1; + return ret; } static int session_gone(void) From 6b8c395e35c6c5d72b3293674f17c71ffd72ceb7 Mon Sep 17 00:00:00 2001 From: Rick Morgans Date: Sat, 14 Mar 2026 23:15:27 +1030 Subject: [PATCH 05/22] fix: treat write()=0 as failure, add regression tests - write_all: return -1 with errno=EIO when write() returns 0 (no progress), preventing an infinite retry loop - Include fault injection tests (preload_short_write.c) that force short socket writes to deterministically verify the retry logic in push and kill paths Co-Authored-By: Claude Opus 4.6 (1M context) --- attach.c | 10 +++++-- tests/preload_short_write.c | 58 +++++++++++++++++++++++++++++++++++++ tests/test.sh | 57 ++++++++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 tests/preload_short_write.c diff --git a/attach.c b/attach.c index f49ac9a..48fcf70 100644 --- a/attach.c +++ b/attach.c @@ -34,13 +34,17 @@ static int write_all(int fd, const void *buf, size_t count) while (count != 0) { ssize_t ret = write(fd, buf, count); - if (ret >= 0) { + if (ret > 0) { buf = (const char *)buf + ret; count -= ret; - } else if (errno == EINTR) + } else if (ret < 0 && errno == EINTR) continue; - else + else { + /* ret == 0 (no progress) or ret < 0 (real error) */ + if (ret == 0) + errno = EIO; return -1; + } } return 0; } diff --git a/tests/preload_short_write.c b/tests/preload_short_write.c new file mode 100644 index 0000000..11828d3 --- /dev/null +++ b/tests/preload_short_write.c @@ -0,0 +1,58 @@ +#include +#include +#include +#include +#include + +static int did_inject; + +static ssize_t real_write(int fd, const void *buf, size_t count) +{ + return syscall(SYS_write, fd, buf, count); +} + +static int should_inject(int fd, size_t count) +{ + struct stat st; + + if (did_inject || count <= 1) + return 0; + if (!getenv("ATCH_FAULT_SHORT_WRITE_ONCE")) + return 0; + if (fstat(fd, &st) < 0) + return 0; + return S_ISSOCK(st.st_mode); +} + +static ssize_t short_write_impl(int fd, const void *buf, size_t count) +{ + if (should_inject(fd, count)) { + did_inject = 1; + return real_write(fd, buf, 1); + } + return real_write(fd, buf, count); +} + +#ifdef __APPLE__ +#define DYLD_INTERPOSE(_replacement, _replacee) \ + __attribute__((used)) static struct { \ + const void *replacement; \ + const void *replacee; \ + } _interpose_##_replacee \ + __attribute__((section("__DATA,__interpose"))) = { \ + (const void *)(unsigned long)&_replacement, \ + (const void *)(unsigned long)&_replacee \ + } + +ssize_t interposed_write(int fd, const void *buf, size_t count) +{ + return short_write_impl(fd, buf, count); +} + +DYLD_INTERPOSE(interposed_write, write); +#else +ssize_t write(int fd, const void *buf, size_t count) +{ + return short_write_impl(fd, buf, count); +} +#endif diff --git a/tests/test.sh b/tests/test.sh index 0c954c0..64b7bc7 100644 --- a/tests/test.sh +++ b/tests/test.sh @@ -720,6 +720,63 @@ assert_contains "no args: shows Usage:" "Usage:" "$out" run "$ATCH" --help assert_contains "help: shows tail command" "tail" "$out" +# ── 23. fault injection: short socket writes are retried ─────────────────── +# Force the first packet write to a socket to complete with 1 byte. +# Verifies write_all() retries correctly instead of treating short writes +# as fatal. + +TESTS_DIR=$(CDPATH= cd -- "$(dirname "$0")" && pwd) +OS_NAME=$(uname -s) + +FAULT_LIB= +build_short_write_injector() { + [ -n "$FAULT_LIB" ] && return 0 + case "$OS_NAME" in + Darwin) + FAULT_LIB="$TESTDIR/libshortwrite.dylib" + cc -dynamiclib -O2 -Wall -o "$FAULT_LIB" \ + "$TESTS_DIR/preload_short_write.c" >/dev/null 2>&1 ;; + *) + FAULT_LIB="$TESTDIR/libshortwrite.so" + cc -shared -fPIC -O2 -Wall -o "$FAULT_LIB" \ + "$TESTS_DIR/preload_short_write.c" -ldl >/dev/null 2>&1 ;; + esac +} + +with_short_socket_write() { + build_short_write_injector || return 1 + case "$OS_NAME" in + Darwin) + env DYLD_INSERT_LIBRARIES="$FAULT_LIB" \ + DYLD_FORCE_FLAT_NAMESPACE=1 \ + ATCH_FAULT_SHORT_WRITE_ONCE=1 "$@" ;; + *) + env LD_PRELOAD="$FAULT_LIB" \ + ATCH_FAULT_SHORT_WRITE_ONCE=1 "$@" ;; + esac +} + +"$ATCH" start short-push sh -c 'cat' +wait_socket short-push +out=$(printf 'short-write-marker\n' | with_short_socket_write \ + "$ATCH" push short-push 2>&1) +prc=$? +assert_exit "fault: push retries short socket write" 0 "$prc" +sleep 0.2 +assert_contains "fault: push data reaches session after short write" \ + "short-write-marker" "$(cat "$HOME/.cache/atch/short-push.log" 2>/dev/null)" +tidy short-push + +"$ATCH" start short-kill sleep 999 +wait_socket short-kill +out=$(with_short_socket_write "$ATCH" kill short-kill 2>&1) +krc=$? +assert_exit "fault: kill retries short socket write" 0 "$krc" +run "$ATCH" list +assert_not_contains "fault: session is gone after short-write kill" \ + "short-kill" "$out" +"$ATCH" kill -f short-kill >/dev/null 2>&1 || true + # ── summary ────────────────────────────────────────────────────────────────── printf "\n1..%d\n" "$T" From adc1181ca7b42a4c914e22a62bcbbd9ffd425996 Mon Sep 17 00:00:00 2001 From: Rick Morgans Date: Sat, 14 Mar 2026 23:26:23 +1030 Subject: [PATCH 06/22] fix: reject SCROLLBACK_SIZE == 0 in static assertion Zero passes the power-of-two bitmask check but is not a valid ring buffer size. Add SCROLLBACK_SIZE > 0 guard. Co-Authored-By: Claude Opus 4.6 (1M context) --- config.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config.h b/config.h index 3b70f94..1e3dfcb 100644 --- a/config.h +++ b/config.h @@ -22,8 +22,8 @@ #ifndef SCROLLBACK_SIZE #define SCROLLBACK_SIZE (128 * 1024) #endif -_Static_assert((SCROLLBACK_SIZE & (SCROLLBACK_SIZE - 1)) == 0, - "SCROLLBACK_SIZE must be a power of two"); +_Static_assert(SCROLLBACK_SIZE > 0 && (SCROLLBACK_SIZE & (SCROLLBACK_SIZE - 1)) == 0, + "SCROLLBACK_SIZE must be a positive power of two"); /* Maximum size of the on-disk session log; older bytes are trimmed on open */ #ifndef LOG_MAX_SIZE From 4d8203473878569421de20ed2e9e99230e4f8189 Mon Sep 17 00:00:00 2001 From: Rick Morgans Date: Sat, 14 Mar 2026 23:29:33 +1030 Subject: [PATCH 07/22] fix: guard all log operations behind log_fd >= 0 Once logging is disabled (log_fd = -1), no code path should increment log_written, call rotate_log, or lseek/write the fd. Co-Authored-By: Claude Opus 4.6 (1M context) --- master.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/master.c b/master.c index 0afaa09..20a5971 100644 --- a/master.c +++ b/master.c @@ -58,6 +58,9 @@ static void rotate_log(void) char *buf; ssize_t n; + if (log_fd < 0) + return; + size = lseek(log_fd, 0, SEEK_END); if (size > (off_t) log_max_size) { buf = malloc(log_max_size); @@ -70,12 +73,15 @@ static void rotate_log(void) if (write(log_fd, buf, (size_t)n) < 0) { close(log_fd); log_fd = -1; + free(buf); + return; } } free(buf); } } - lseek(log_fd, 0, SEEK_END); + if (log_fd >= 0) + lseek(log_fd, 0, SEEK_END); } /* @@ -92,7 +98,7 @@ static int open_log(const char *path) log_fd = fd; rotate_log(); - return fd; + return log_fd; } /* Write end marker to log, close it, and unlink the socket. */ @@ -395,6 +401,8 @@ static void pty_activity(int s) close(log_fd); log_fd = -1; } + } + if (log_fd >= 0) { log_written += (size_t)len; if (log_written >= log_max_size) { rotate_log(); From 0f0f734d86748a119a78c0a9726dbafb25b3ecd8 Mon Sep 17 00:00:00 2001 From: Rick Morgans Date: Sun, 15 Mar 2026 11:09:46 +1030 Subject: [PATCH 08/22] fix: make signal handlers async-signal-safe and handle blocked writes Replace printf/exit in signal handlers with volatile sig_atomic_t flags. Use sigaction() instead of signal() to control SA_RESTART explicitly: SA_RESTART for SIGWINCH (benign), no SA_RESTART for fatal signals so select() returns EINTR promptly. Handle EINTR in read paths. write_all() stops retrying EINTR when die_signal is set. When stdout itself is wedged (blocked write to PTY), the deferred-signal exit path uses TCSANOW + _exit() to avoid hanging in printf or atexit handlers. Co-Authored-By: Claude Opus 4.6 (1M context) --- attach.c | 96 +++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 71 insertions(+), 25 deletions(-) diff --git a/attach.c b/attach.c index 48fcf70..55f183e 100644 --- a/attach.c +++ b/attach.c @@ -14,7 +14,9 @@ */ static struct termios cur_term; /* 1 if the window size changed */ -static int win_changed; +static volatile sig_atomic_t win_changed; +/* Non-zero if a fatal signal was received; stores the signal number. */ +static volatile sig_atomic_t die_signal; /* Socket creation time, used to compute session age in messages. */ time_t session_start; @@ -27,6 +29,29 @@ char const *clear_csi_data(void) return "\033[999H\r\n"; } +/* Exit promptly once the main thread notices a fatal signal. + * If terminal output itself is wedged, skip stdio entirely. */ +static void exit_for_deferred_signal(int can_print) +{ + int sig = die_signal; + char age[32]; + + if (!sig) + return; + if (!can_print) { + tcsetattr(0, TCSANOW, &orig_term); + _exit(1); + } + session_age(age, sizeof(age)); + if (sig == SIGHUP || sig == SIGINT) + printf("%s[%s: session '%s' detached after %s]\r\n", + clear_csi_data(), progname, session_shortname(), age); + else + printf("%s[%s: session '%s' got signal %d - exiting after %s]\r\n", + clear_csi_data(), progname, session_shortname(), sig, age); + exit(1); +} + /* Write all of buf to fd, retrying on short writes and EINTR. ** Returns 0 on success, -1 on failure (errno is set). */ static int write_all(int fd, const void *buf, size_t count) @@ -37,9 +62,11 @@ static int write_all(int fd, const void *buf, size_t count) if (ret > 0) { buf = (const char *)buf + ret; count -= ret; - } else if (ret < 0 && errno == EINTR) + } else if (ret < 0 && errno == EINTR) { + if (die_signal) + return -1; continue; - else { + } else { /* ret == 0 (no progress) or ret < 0 (real error) */ if (ret == 0) errno = EIO; @@ -53,6 +80,7 @@ static int write_all(int fd, const void *buf, size_t count) void write_buf_or_fail(int fd, const void *buf, size_t count) { if (write_all(fd, buf, count) < 0) { + exit_for_deferred_signal(fd != 1); if (session_start) { char age[32]; session_age(age, sizeof(age)); @@ -72,6 +100,7 @@ void write_buf_or_fail(int fd, const void *buf, size_t count) void write_packet_or_fail(int fd, const struct packet *pkt) { if (write_all(fd, pkt, sizeof(struct packet)) < 0) { + exit_for_deferred_signal(fd != 1); if (session_start) { char age[32]; session_age(age, sizeof(age)); @@ -154,26 +183,15 @@ void session_age(char *buf, size_t size) format_age(now > session_start ? now - session_start : 0, buf, size); } -/* Signal */ +/* Signal -- only set a flag; all non-trivial work happens in the main loop. */ static RETSIGTYPE die(int sig) { - char age[32]; - session_age(age, sizeof(age)); - /* Print a nice pretty message for some things. */ - if (sig == SIGHUP || sig == SIGINT) - printf("%s[%s: session '%s' detached after %s]\r\n", - clear_csi_data(), progname, session_shortname(), age); - else - printf - ("%s[%s: session '%s' got signal %d - exiting after %s]\r\n", - clear_csi_data(), progname, session_shortname(), sig, age); - exit(1); + die_signal = sig; } -/* Window size change. */ +/* Window size change -- only set a flag. */ static RETSIGTYPE win_change(ATTRIBUTE_UNUSED int sig) { - signal(SIGWINCH, win_change); win_changed = 1; } @@ -349,14 +367,32 @@ int attach_main(int noerror) /* Set a trap to restore the terminal when we die. */ atexit(restore_term); - /* Set some signals. */ - signal(SIGPIPE, SIG_IGN); - signal(SIGXFSZ, SIG_IGN); - signal(SIGHUP, die); - signal(SIGTERM, die); - signal(SIGINT, die); - signal(SIGQUIT, die); - signal(SIGWINCH, win_change); + /* Set some signals using sigaction to avoid SA_RESTART ambiguity. */ + { + struct sigaction sa_ign, sa_die, sa_winch; + + memset(&sa_ign, 0, sizeof(sa_ign)); + sa_ign.sa_handler = SIG_IGN; + sigemptyset(&sa_ign.sa_mask); + sigaction(SIGPIPE, &sa_ign, NULL); + sigaction(SIGXFSZ, &sa_ign, NULL); + + memset(&sa_die, 0, sizeof(sa_die)); + sa_die.sa_handler = die; + sigemptyset(&sa_die.sa_mask); + /* No SA_RESTART: let select() return EINTR so the loop + * notices die_signal promptly. */ + sigaction(SIGHUP, &sa_die, NULL); + sigaction(SIGTERM, &sa_die, NULL); + sigaction(SIGINT, &sa_die, NULL); + sigaction(SIGQUIT, &sa_die, NULL); + + memset(&sa_winch, 0, sizeof(sa_winch)); + sa_winch.sa_handler = win_change; + sigemptyset(&sa_winch.sa_mask); + sa_winch.sa_flags = SA_RESTART; /* benign — don't interrupt I/O */ + sigaction(SIGWINCH, &sa_winch, NULL); + } /* Set raw mode. */ cur_term.c_iflag &= @@ -401,10 +437,16 @@ int attach_main(int noerror) while (1) { int n; + exit_for_deferred_signal(1); + FD_ZERO(&readfds); FD_SET(0, &readfds); FD_SET(s, &readfds); n = select(s + 1, &readfds, NULL, NULL, NULL); + + /* Check for deferred fatal signal. */ + exit_for_deferred_signal(1); + if (n < 0 && errno != EINTR && errno != EAGAIN) { char age[32]; session_age(age, sizeof(age)); @@ -430,6 +472,8 @@ int attach_main(int noerror) } exit(0); } else if (len < 0) { + if (errno == EINTR) + continue; char age[32]; session_age(age, sizeof(age)); printf @@ -450,6 +494,8 @@ int attach_main(int noerror) memset(pkt.u.buf, 0, sizeof(pkt.u.buf)); len = read(0, pkt.u.buf, sizeof(pkt.u.buf)); + if (len < 0 && errno == EINTR) + continue; if (len <= 0) exit(1); From fea3204a2c794e1a1c2b560f90b774a5f19aca1f Mon Sep 17 00:00:00 2001 From: Rick Morgans Date: Sun, 15 Mar 2026 11:09:55 +1030 Subject: [PATCH 09/22] test: add forkpty-based signal safety integration tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C harness using forkpty() for exact PID targeting — no pkill races or heuristic process identification. Tests SIGWINCH survival, SIGTERM exit, SIGHUP exit, SIGTERM during blocked stdout write, and detach character. Wire harness into test.sh with TAP folding that preserves diagnostic lines for debuggability. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/test.sh | 37 +++++ tests/test_signal.c | 381 +++++++++++++++++++++++++++++++++++++++++++ tests/test_signal.sh | 50 ++++++ 3 files changed, 468 insertions(+) create mode 100644 tests/test_signal.c create mode 100755 tests/test_signal.sh diff --git a/tests/test.sh b/tests/test.sh index 64b7bc7..1578859 100644 --- a/tests/test.sh +++ b/tests/test.sh @@ -777,6 +777,43 @@ assert_not_contains "fault: session is gone after short-write kill" \ "short-kill" "$out" "$ATCH" kill -f short-kill >/dev/null 2>&1 || true +# ── 24. signal safety (forkpty harness) ──────────────────────────────────── +# Builds and runs a C test binary that uses forkpty() to send signals +# to the exact atch attach PID. Skips gracefully if cc is unavailable. + +TESTS_DIR=$(CDPATH= cd -- "$(dirname "$0")" && pwd) +SIGNAL_HARNESS="$TESTDIR/test_signal" + +if cc -o "$SIGNAL_HARNESS" "$TESTS_DIR/test_signal.c" -lutil 2>/dev/null; then + "$ATCH" start sig-harness sleep 9999 + wait_socket sig-harness + + sig_out=$("$SIGNAL_HARNESS" "$ATCH" sig-harness 2>&1) + + # Fold harness results into main TAP stream (avoid subshell pipe) + sig_tmpfile="$TESTDIR/sig_out.txt" + echo "$sig_out" > "$sig_tmpfile" + while IFS= read -r line; do + case "$line" in + ok\ *) + desc=$(echo "$line" | sed 's/^ok [0-9]* - //') + ok "signal: $desc" + ;; + not\ ok\ *) + desc=$(echo "$line" | sed 's/^not ok [0-9]* - //') + fail "signal: $desc" + ;; + "#"*) + printf "%s\n" "$line" + ;; + esac + done < "$sig_tmpfile" + + tidy sig-harness +else + ok "signal: SKIP — cc not available, cannot build forkpty harness" +fi + # ── summary ────────────────────────────────────────────────────────────────── printf "\n1..%d\n" "$T" diff --git a/tests/test_signal.c b/tests/test_signal.c new file mode 100644 index 0000000..813e56b --- /dev/null +++ b/tests/test_signal.c @@ -0,0 +1,381 @@ +/* + * test_signal.c — deterministic signal-safety tests for atch attach. + * + * Uses forkpty() to create a real PTY, execs atch attach in the child, + * and sends signals to the exact child PID from the parent. No pkill, + * no script, no heuristics. + * + * Build: cc -o test_signal tests/test_signal.c -lutil + * Usage: ./test_signal + * + * Requires a running atch session named "sig-test-session". + * The wrapper script tests/test_signal.sh handles setup/teardown. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#if defined(__APPLE__) +#include +#else +#include +#endif + +static int pass_count = 0; +static int fail_count = 0; +static int test_num = 0; + +static void ok(const char *desc) +{ + test_num++; + pass_count++; + printf("ok %d - %s\n", test_num, desc); +} + +static void fail(const char *desc, const char *detail) +{ + test_num++; + fail_count++; + printf("not ok %d - %s\n", test_num, desc); + if (detail) + printf(" # %s\n", detail); +} + +/* Wait for child to exit within timeout_ms, draining master_fd to prevent + * the child from blocking on PTY writes (e.g. during session log replay). + * Pass master_fd=-1 if the master is already closed. + * Returns exit status or -1 on timeout. */ +static int wait_exit(pid_t pid, int timeout_ms, int master_fd) +{ + int elapsed = 0; + int status; + char drain[4096]; + + while (elapsed < timeout_ms) { + pid_t r = waitpid(pid, &status, WNOHANG); + if (r == pid) { + if (WIFEXITED(status)) + return WEXITSTATUS(status); + if (WIFSIGNALED(status)) + return 128 + WTERMSIG(status); + return -1; + } + /* Drain PTY output to prevent child blocking on write */ + if (master_fd >= 0) { + struct pollfd pfd = { .fd = master_fd, .events = POLLIN }; + if (poll(&pfd, 1, 0) > 0) + (void)read(master_fd, drain, sizeof(drain)); + } + usleep(10000); /* 10ms */ + elapsed += 10; + } + return -1; /* timed out */ +} + +/* Check if pid is still alive. */ +static int is_alive(pid_t pid) +{ + return kill(pid, 0) == 0; +} + +/* Read available data from fd into buf (non-blocking). */ +static ssize_t drain_fd(int fd, char *buf, size_t size, int timeout_ms) +{ + struct pollfd pfd = { .fd = fd, .events = POLLIN }; + ssize_t total = 0; + + while (total < (ssize_t)size - 1) { + int r = poll(&pfd, 1, timeout_ms); + if (r <= 0) + break; + ssize_t n = read(fd, buf + total, size - 1 - total); + if (n <= 0) + break; + total += n; + timeout_ms = 50; /* short timeout for subsequent reads */ + } + buf[total] = '\0'; + return total; +} + +/* + * Fork a child that execs atch attach . + * Returns child PID, sets *master_fd to the PTY master. + */ +static pid_t spawn_attach(const char *atch_bin, const char *session, + int *master_fd) +{ + int master; + pid_t pid = forkpty(&master, NULL, NULL, NULL); + + if (pid < 0) { + perror("forkpty"); + exit(1); + } + + if (pid == 0) { + /* child — exec atch attach */ + execl(atch_bin, atch_bin, "attach", session, (char *)NULL); + perror("execl"); + _exit(127); + } + + /* parent */ + *master_fd = master; + + /* Make master non-blocking for drain_fd */ + int flags = fcntl(master, F_GETFL); + if (flags >= 0) + fcntl(master, F_SETFL, flags | O_NONBLOCK); + + return pid; +} + +/* + * Test: SIGWINCH does not kill the attach process. + * After receiving SIGWINCH, the child must still be alive. + */ +static void test_sigwinch_survives(const char *atch_bin, const char *session) +{ + int master; + pid_t pid = spawn_attach(atch_bin, session, &master); + + /* Let attach settle */ + usleep(300000); + + if (!is_alive(pid)) { + fail("sigwinch: child alive before signal", "child died during attach"); + close(master); + return; + } + + /* Send SIGWINCH */ + kill(pid, SIGWINCH); + usleep(200000); + + if (is_alive(pid)) + ok("sigwinch: child survives SIGWINCH"); + else + fail("sigwinch: child survives SIGWINCH", "child died after SIGWINCH"); + + /* Send burst of SIGWINCH */ + for (int i = 0; i < 10; i++) { + kill(pid, SIGWINCH); + usleep(10000); + } + usleep(200000); + + if (is_alive(pid)) + ok("sigwinch: child survives SIGWINCH burst (10x)"); + else + fail("sigwinch: child survives SIGWINCH burst", "child died during burst"); + + /* Clean up: send SIGTERM to exit */ + kill(pid, SIGTERM); + wait_exit(pid, 2000, master); + close(master); +} + +/* + * Test: SIGTERM causes prompt exit (no deadlock). + */ +static void test_sigterm_exits(const char *atch_bin, const char *session) +{ + int master; + pid_t pid = spawn_attach(atch_bin, session, &master); + + usleep(300000); + + if (!is_alive(pid)) { + fail("sigterm: child alive before signal", "child died during attach"); + close(master); + return; + } + + kill(pid, SIGTERM); + + /* Wait with PTY master still open — child must exit from the signal + * handler, not from EOF on the terminal. Parent drains the PTY to + * prevent the child blocking on replay writes. */ + int status = wait_exit(pid, 3000, master); + close(master); + + if (status == -1) { + fail("sigterm: child exits within 3s (master still open)", + "child hung (possible deadlock)"); + kill(pid, SIGKILL); + waitpid(pid, NULL, 0); + } else { + ok("sigterm: child exits promptly after SIGTERM"); + } + + if (!is_alive(pid)) + ok("sigterm: child is dead after SIGTERM"); + else + fail("sigterm: child is dead after SIGTERM", "child still alive"); +} + +/* + * Test: SIGHUP causes prompt exit (simulates SSH disconnect). + */ +static void test_sighup_exits(const char *atch_bin, const char *session) +{ + int master; + pid_t pid = spawn_attach(atch_bin, session, &master); + + usleep(300000); + + if (!is_alive(pid)) { + fail("sighup: child alive before signal", "child died during attach"); + close(master); + return; + } + + kill(pid, SIGHUP); + + /* Wait with PTY master still open — child must exit from the signal + * handler, not from EOF on the terminal. Parent drains the PTY to + * prevent the child blocking on replay writes. */ + int status = wait_exit(pid, 3000, master); + close(master); + + if (status == -1) { + fail("sighup: child exits within 3s (master still open)", + "child hung (possible deadlock)"); + kill(pid, SIGKILL); + waitpid(pid, NULL, 0); + } else { + ok("sighup: child exits promptly after SIGHUP"); + } + + if (!is_alive(pid)) + ok("sighup: child is dead after SIGHUP"); + else + fail("sighup: child is dead after SIGHUP", "child still alive"); +} + +/* + * Test: SIGTERM still exits promptly when attach is blocked writing to + * its own PTY. The parent intentionally does not drain the PTY master. + */ +static void test_sigterm_exits_while_stdout_blocked(const char *atch_bin, + const char *session) +{ + int master; + pid_t pid = spawn_attach(atch_bin, session, &master); + + /* Give the noisy session time to fill the PTY and block the child. */ + usleep(1000000); + + if (!is_alive(pid)) { + fail("sigterm: child alive before blocked-write signal", + "child died during noisy attach"); + close(master); + return; + } + + kill(pid, SIGTERM); + + /* Keep the PTY master open but undrained. If attach retries EINTR + * forever in write_all(), this wait will time out. */ + int status = wait_exit(pid, 3000, -1); + close(master); + + if (status == -1) { + fail("sigterm: child exits within 3s while stdout blocked", + "child hung in blocked write"); + kill(pid, SIGKILL); + waitpid(pid, NULL, 0); + } else { + ok("sigterm: child exits promptly while stdout blocked"); + } + + if (!is_alive(pid)) + ok("sigterm: child is dead after blocked-write SIGTERM"); + else + fail("sigterm: child is dead after blocked-write SIGTERM", + "child still alive"); +} + +/* + * Test: detach character (^\, 0x1c) causes clean detach. + * The child should exit, and the session should still be running. + */ +static void test_detach_char(const char *atch_bin, const char *session) +{ + int master; + pid_t pid = spawn_attach(atch_bin, session, &master); + + usleep(300000); + + if (!is_alive(pid)) { + fail("detach: child alive before detach char", "child died during attach"); + close(master); + return; + } + + /* Send detach character: ^\ (0x1c) */ + char detach = 0x1c; + if (write(master, &detach, 1) < 0) { + fail("detach: write detach char to PTY", strerror(errno)); + close(master); + return; + } + + /* Give atch time to process the detach and exit */ + usleep(500000); + + /* Read any output from PTY before closing */ + char buf[4096]; + drain_fd(master, buf, sizeof(buf), 200); + close(master); + + int status = wait_exit(pid, 3000, -1); + if (status == -1) { + fail("detach: child exits after detach char", "child hung"); + kill(pid, SIGKILL); + waitpid(pid, NULL, 0); + } else { + ok("detach: child exits after detach char"); + } + + if (!is_alive(pid)) + ok("detach: child is dead after detach"); + else + fail("detach: child is dead after detach", "child still alive"); +} + +int main(int argc, char **argv) +{ + if (argc < 2) { + fprintf(stderr, + "Usage: %s [session-name] [noisy-session]\n", + argv[0]); + return 1; + } + + const char *atch_bin = argv[1]; + const char *session = argc > 2 ? argv[2] : "sig-test-session"; + const char *noisy_session = argc > 3 ? argv[3] : "sig-noisy-session"; + + printf("TAP version 13\n"); + + test_sigwinch_survives(atch_bin, session); + test_sigterm_exits(atch_bin, session); + test_sighup_exits(atch_bin, session); + test_sigterm_exits_while_stdout_blocked(atch_bin, noisy_session); + test_detach_char(atch_bin, session); + + printf("\n1..%d\n", test_num); + printf("# %d passed, %d failed\n", pass_count, fail_count); + + return fail_count > 0 ? 1 : 0; +} diff --git a/tests/test_signal.sh b/tests/test_signal.sh new file mode 100755 index 0000000..3df1e15 --- /dev/null +++ b/tests/test_signal.sh @@ -0,0 +1,50 @@ +#!/bin/sh +# Signal-safety integration tests for atch attach. +# Uses a forkpty()-based C harness for exact PID targeting. +# +# Usage: sh tests/test_signal.sh +# Builds the test harness automatically if needed. + +ATCH="${1:-./atch}" +TESTS_DIR=$(CDPATH= cd -- "$(dirname "$0")" && pwd) +TESTDIR=$(mktemp -d) +export HOME="$TESTDIR" +SESSION="sig-test-session" +NOISY_SESSION="sig-noisy-session" + +trap '"$ATCH" kill "$NOISY_SESSION" >/dev/null 2>&1 || true; "$ATCH" kill "$SESSION" >/dev/null 2>&1 || true; rm -rf "$TESTDIR"' EXIT + +# Build the harness (graceful skip if cc fails) +HARNESS="$TESTDIR/test_signal" +if ! cc -o "$HARNESS" "$TESTS_DIR/test_signal.c" -lutil 2>/dev/null; then + echo "1..0 # SKIP cannot build forkpty harness" + exit 0 +fi + +# Start a background session for the tests to attach to +"$ATCH" start "$SESSION" sleep 9999 || { + echo "1..0 # SKIP failed to start test session" + exit 0 +} + +"$ATCH" start "$NOISY_SESSION" sh -c 'yes X' || { + echo "1..0 # SKIP failed to start noisy test session" + exit 0 +} + +# Wait for socket +i=0 +while [ $i -lt 20 ]; do + [ -S "$HOME/.cache/atch/$SESSION" ] && break + sleep 0.05 + i=$((i + 1)) +done + +if [ ! -S "$HOME/.cache/atch/$SESSION" ] || + [ ! -S "$HOME/.cache/atch/$NOISY_SESSION" ]; then + echo "1..0 # SKIP session socket did not appear" + exit 0 +fi + +# Run the harness +"$HARNESS" "$ATCH" "$SESSION" "$NOISY_SESSION" From 92a953289e8d470186744042afa585a4d9c499c1 Mon Sep 17 00:00:00 2001 From: Rick Morgans Date: Sun, 15 Mar 2026 11:10:08 +1030 Subject: [PATCH 10/22] chore: ignore *.dSYM directories in .gitignore Co-Authored-By: Claude Opus 4.6 (1M context) --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 420cca7..6fd430d 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ /.* !.gitignore *.o +*.dSYM/ *~ *.1 *.1.md From b9f3aa3d461b9c6476cde7f33d36adca587ae3ff Mon Sep 17 00:00:00 2001 From: Donaldo Date: Tue, 10 Mar 2026 16:39:47 +0100 Subject: [PATCH 11/22] fix(master): use umask(0177) in create_socket to prevent S_IXUSR race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit create_socket restored the original umask before calling bind(2). With a typical shell umask of 022, bind created the socket file with mode 0755 (S_IXUSR set). chmod(0600) was called right after, but the tiny window between bind and chmod was enough for a concurrent `atch list` to see the stale execute bit and report a freshly started session as [attached] — even though no client had ever connected. The fix switches to umask(0177) before bind so the kernel creates the socket file directly at mode 0600 (0777 & ~0177). S_IXUSR is never present on the socket path during creation, closing the TOCTOU window entirely. The subsequent chmod(0600) is kept for explicitness and to guard against any platform-specific deviations. Adds regression test 24 (start-inside-session) that: - starts an outer session and attaches a python client to it - starts an inner session with ATCH_SESSION set (simulating being inside the outer session) - asserts the inner session socket has no execute bit immediately after creation - asserts `atch list` does not show the inner session as [attached] Co-Authored-By: Claude Sonnet 4.6 --- master.c | 19 ++++++++++--- tests/test.sh | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 4 deletions(-) diff --git a/master.c b/master.c index 16f29e3..9a9ad7d 100644 --- a/master.c +++ b/master.c @@ -246,17 +246,28 @@ static int create_socket(char *name) if (strlen(name) > sizeof(sockun.sun_path) - 1) return socket_with_chdir(name, create_socket); - omask = umask(077); + /* + ** Use umask(0177) during bind so the kernel creates the socket file + ** with mode 0600 directly (0777 & ~0177 = 0600). This ensures + ** S_IXUSR is never set on the socket file at any point during + ** creation, eliminating the TOCTOU window between bind(2) and the + ** subsequent chmod(2) that would otherwise let `atch list` briefly + ** see a newly-started session as [attached]. + */ + omask = umask(0177); s = socket(PF_UNIX, SOCK_STREAM, 0); - umask(omask); /* umask always succeeds, errno is untouched. */ - if (s < 0) + if (s < 0) { + umask(omask); return -1; + } sockun.sun_family = AF_UNIX; memcpy(sockun.sun_path, name, strlen(name) + 1); if (bind(s, (struct sockaddr *)&sockun, sizeof(sockun)) < 0) { + umask(omask); close(s); return -1; } + umask(omask); /* umask always succeeds, errno is untouched. */ if (listen(s, 128) < 0) { close(s); return -1; @@ -265,7 +276,7 @@ static int create_socket(char *name) close(s); return -1; } - /* chmod it to prevent any surprises */ + /* chmod it to enforce 0600 regardless of platform quirks */ if (chmod(name, 0600) < 0) { close(s); return -1; diff --git a/tests/test.sh b/tests/test.sh index 0c954c0..116830f 100644 --- a/tests/test.sh +++ b/tests/test.sh @@ -720,6 +720,84 @@ assert_contains "no args: shows Usage:" "Usage:" "$out" run "$ATCH" --help assert_contains "help: shows tail command" "tail" "$out" +# ── 22. start-inside-session: no [attached] when started from inside a session ── +# +# Regression test for: a session created with `atch start` from within an +# attached session must never appear as [attached] in `atch list`. +# +# Root cause: create_socket restored the original umask BEFORE calling bind(2). +# With a typical shell umask of 022, bind created the socket file with mode +# 0755 (S_IXUSR set). chmod(0600) was called immediately after, but the +# tiny window between bind and chmod was enough for a concurrent `atch list` +# (or an immediate stat after start) to see the stale execute bit and report +# the session as [attached]. +# +# Fix: use umask(0177) before bind so the socket is created directly as 0600 +# (no execute bit ever present during creation). +# +# Test strategy: +# A. Start outer-session so there is an [attached] session in the directory. +# B. Simulate being inside outer-session by setting ATCH_SESSION. +# C. Run `atch start inner-session` — no client must ever attach. +# D. Check socket mode immediately: S_IXUSR must NOT be set. +# E. Check `atch list`: inner-session must NOT show [attached]. + +"$ATCH" start sis-outer sleep 999 +wait_socket sis-outer +SIS_OUTER_SOCK="$HOME/.cache/atch/sis-outer" + +# Attach to outer via python so it shows [attached] — this mirrors the real +# scenario where the user is inside the outer session. +if command -v python3 >/dev/null 2>&1; then + python3 - "$SIS_OUTER_SOCK" << 'PYEOF' & +import socket, struct, sys, time +s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) +s.connect(sys.argv[1]) +s.sendall(struct.pack('BB8s', 1, 0, b'\x00' * 8)) # MSG_ATTACH +time.sleep(15) +s.close() +PYEOF + SIS_ATTACH_PID=$! + sleep 0.1 + + # Start inner-session as if we are inside outer-session (ATCH_SESSION set) + ATCH_SESSION="$SIS_OUTER_SOCK" "$ATCH" start sis-inner sleep 999 + wait_socket sis-inner + SIS_INNER_SOCK="$HOME/.cache/atch/sis-inner" + + # Check socket mode immediately after start: no S_IXUSR allowed. + # The owner execute bit (S_IXUSR) is the bit 0 of the hundreds digit + # in the 3-digit octal representation (i.e., digit is 1, 3, 5, or 7). + # We extract the hundreds digit and test whether it is odd. + SOCK_MODE=$(stat -c "%a" "$SIS_INNER_SOCK" 2>/dev/null || \ + stat -f "%Lp" "$SIS_INNER_SOCK" 2>/dev/null || echo "unknown") + # Hundreds digit: remove last two chars → first char of 3-digit mode + OWNER_DIGIT="${SOCK_MODE%??}" + case "$OWNER_DIGIT" in + 1|3|5|7) + fail "start-inside: socket mode must not have S_IXUSR" \ + "owner digit 0,2,4 or 6 (no execute)" "$OWNER_DIGIT (mode $SOCK_MODE)" ;; + *) + ok "start-inside: socket created without S_IXUSR (mode $SOCK_MODE)" ;; + esac + + # Check list: inner-session must NOT appear as [attached] + run "$ATCH" list + assert_not_contains \ + "start-inside: inner session not shown as [attached] in list" \ + "[attached]" \ + "$(echo "$out" | grep sis-inner)" + + kill $SIS_ATTACH_PID 2>/dev/null + wait $SIS_ATTACH_PID 2>/dev/null + + tidy sis-outer + tidy sis-inner +else + ok "start-inside: skip (python3 not available)" + ok "start-inside: skip (python3 not available)" +fi + # ── summary ────────────────────────────────────────────────────────────────── printf "\n1..%d\n" "$T" From 31769a0b22cb58036310ce604f3b2089bde2464c Mon Sep 17 00:00:00 2001 From: Donaldo Date: Tue, 10 Mar 2026 16:14:36 +0100 Subject: [PATCH 12/22] fix(attach): send MSG_DETACH before exit on detach_char press MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Ctrl+\ was pressed, process_kbd called exit(0) without notifying the master via MSG_DETACH. The master only learned about the detach when it received EOF on the closed fd, introducing a window where a concurrent `atch list` could read the stale S_IXUSR bit and display "[attached]" for a session that was already detached. Send MSG_DETACH synchronously before calling exit(0), mirroring the existing suspend-key (VSUSP) path. This ensures the master clears the S_IXUSR mode bit within one select cycle — before the client exits. Adds regression tests (test 23) exercising the MSG_DETACH → list flow across single and multi-session attach/detach cycles. Co-Authored-By: Claude Sonnet 4.6 --- attach.c | 7 ++++ tests/test.sh | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+) diff --git a/attach.c b/attach.c index 2de74ac..d363862 100644 --- a/attach.c +++ b/attach.c @@ -203,6 +203,13 @@ static void process_kbd(int s, struct packet *pkt) else if (pkt->u.buf[0] == detach_char) { char age[32]; session_age(age, sizeof(age)); + /* Tell the master we are detaching so it clears S_IXUSR on + * the socket immediately, before this process exits. + * Without this, the master only learns about the detach when + * it receives EOF on close(), which can race with a concurrent + * `atch list` reading the stale S_IXUSR bit. */ + pkt->type = MSG_DETACH; + write_packet_or_fail(s, pkt); printf("%s[%s: session '%s' detached after %s]\r\n", clear_csi_data(), progname, session_shortname(), age); exit(0); diff --git a/tests/test.sh b/tests/test.sh index 116830f..0b4af1b 100644 --- a/tests/test.sh +++ b/tests/test.sh @@ -798,6 +798,109 @@ else ok "start-inside: skip (python3 not available)" fi +# ── 23. detach-status: S_IXUSR cleared immediately after MSG_DETACH ────────── +# +# Regression test for: when the client detaches (Ctrl+\), it must send +# MSG_DETACH to the master BEFORE calling exit(0). This ensures the master +# clears the S_IXUSR bit on the socket synchronously (within one select cycle) +# so that `atch list` never races with a stale "[attached]" status. +# +# Without the fix, the client exits without MSG_DETACH; the master only learns +# about the detach when it receives EOF on the closed fd, which can arrive after +# a `list` reads the stale S_IXUSR bit — especially on loaded systems. +# +# Strategy: use Python to simulate the two scenarios: +# A. MSG_DETACH sent before close → socket must lose S_IXUSR immediately +# B. Close without MSG_DETACH → socket loses S_IXUSR after one master +# select cycle (tolerated, but slower) +# +# The critical invariant tested here is scenario A: after MSG_DETACH is sent +# and acknowledged, `list` must NOT show "[attached]". This is the exact +# behaviour enforced by the fix in process_kbd. + +if command -v python3 >/dev/null 2>&1; then + + # Helper: send MSG_ATTACH, optionally MSG_DETACH, then close. + # Usage: attach_and_detach + attach_and_detach() { + python3 - "$1" "$2" << 'PYEOF' +import socket, struct, sys, time + +sock_path = sys.argv[1] +send_detach = sys.argv[2] == '1' + +MSG_ATTACH = 1 +MSG_DETACH = 2 + +def pkt(msg_type): + return struct.pack('BB8s', msg_type, 0, b'\x00' * 8) + +s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) +s.connect(sock_path) +s.sendall(pkt(MSG_ATTACH)) +time.sleep(0.05) # let master process MSG_ATTACH and set S_IXUSR +if send_detach: + s.sendall(pkt(MSG_DETACH)) + time.sleep(0.05) # let master process MSG_DETACH and clear S_IXUSR +s.close() +PYEOF + } + + # --- single session: proper MSG_DETACH flow (scenario A) --- + "$ATCH" start det-s1 sleep 999 + wait_socket det-s1 + SOCK1="$HOME/.cache/atch/det-s1" + + attach_and_detach "$SOCK1" 1 # send MSG_DETACH before close + sleep 0.05 # minimal delay after close + + run "$ATCH" list + assert_not_contains \ + "detach-status: session not shown as attached after MSG_DETACH" \ + "[attached]" "$out" + + tidy det-s1 + + # --- two sessions: reproduce the multi-session attach/detach cycle --- + # Steps mirror the exact reproduction sequence from the bug report: + # create s1, detach, create s2, detach, + # attach s1, detach, attach s2, detach → none should show [attached] + "$ATCH" start det-a sleep 999 + "$ATCH" start det-b sleep 999 + wait_socket det-a + wait_socket det-b + SOCKA="$HOME/.cache/atch/det-a" + SOCKB="$HOME/.cache/atch/det-b" + + attach_and_detach "$SOCKA" 1 + sleep 0.05 + attach_and_detach "$SOCKB" 1 + sleep 0.05 + attach_and_detach "$SOCKA" 1 + sleep 0.05 + + run "$ATCH" list + assert_not_contains \ + "detach-status: det-a not [attached] after second detach cycle" \ + "[attached]" "$out" + + attach_and_detach "$SOCKB" 1 + sleep 0.05 + + run "$ATCH" list + assert_not_contains \ + "detach-status: det-b not [attached] after detach cycle" \ + "[attached]" "$out" + + tidy det-a + tidy det-b + +else + ok "detach-status: skip (python3 not available)" + ok "detach-status: skip (python3 not available)" + ok "detach-status: skip (python3 not available)" +fi + # ── summary ────────────────────────────────────────────────────────────────── printf "\n1..%d\n" "$T" From dbd04ec9d1a25daf64c19e1c125ff8fc5d47e1b7 Mon Sep 17 00:00:00 2001 From: Donaldo Date: Tue, 10 Mar 2026 15:50:19 +0100 Subject: [PATCH 13/22] fix(attach): cap session log replay to last 128KB Large session logs (>1MB) caused an apparent infinite scroll on reattach. Seek to the last SCROLLBACK_SIZE bytes before replaying, consistent with the in-memory ring buffer size. Co-Authored-By: Claude Opus 4.6 --- attach.c | 19 ++++++++++++- tests/test.sh | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/attach.c b/attach.c index d363862..22006ff 100644 --- a/attach.c +++ b/attach.c @@ -231,7 +231,12 @@ static int log_already_replayed; ** killed/crashed (socket still on disk), ENOENT means clean exit (socket was ** unlinked; end marker is already in the log). ** Pass 0 when replaying for a running session (no end message printed). -** Returns 1 if a log was found and replayed, 0 if no log exists. */ +** Returns 1 if a log was found and replayed, 0 if no log exists. +** +** Only the last SCROLLBACK_SIZE bytes of the log are replayed to avoid +** overwhelming the terminal when attaching to a session with a large log +** (e.g. a long-running build). This matches the in-memory ring-buffer cap +** used when replaying a live session's scrollback. */ int replay_session_log(int saved_errno) { char log_path[600]; @@ -246,6 +251,18 @@ int replay_session_log(int saved_errno) { unsigned char rbuf[BUFSIZE]; ssize_t n; + off_t log_size; + + /* Seek to the last SCROLLBACK_SIZE bytes so that a very large + * log (e.g. from a long build session) does not flood the + * terminal. If the log is smaller than SCROLLBACK_SIZE, start + * from the beginning. */ + log_size = lseek(logfd, 0, SEEK_END); + if (log_size > (off_t)SCROLLBACK_SIZE) + lseek(logfd, log_size - (off_t)SCROLLBACK_SIZE, + SEEK_SET); + else + lseek(logfd, 0, SEEK_SET); while ((n = read(logfd, rbuf, sizeof(rbuf))) > 0) write(1, rbuf, (size_t)n); diff --git a/tests/test.sh b/tests/test.sh index 0b4af1b..52e1003 100644 --- a/tests/test.sh +++ b/tests/test.sh @@ -708,6 +708,82 @@ assert_contains "tail -n missing arg: message" "-n requires an argument" " tidy s-tail +# ── 21. replay_session_log: bounded replay (last SCROLLBACK_SIZE bytes only) ── +# +# Regression test for: replay_session_log must replay at most SCROLLBACK_SIZE +# (128 KB) of the session log. Without this cap, attaching a session with a +# large log (e.g. a long-running build) causes an overwhelming scroll that +# appears to loop indefinitely. +# +# Strategy: create a synthetic .log file larger than SCROLLBACK_SIZE (128 KB), +# attach to the dead session using expect(1) to supply a PTY (required by +# attach_main), and verify the output byte count and content. +# +# expect(1) is available on macOS by default and on most Linux distros. +# If absent, the test is skipped. + +if command -v expect >/dev/null 2>&1 && command -v python3 >/dev/null 2>&1; then + mkdir -p "$HOME/.cache/atch" + + REPLAY_SOCK="$HOME/.cache/atch/replay-cap-sess" + REPLAY_LOG="${REPLAY_SOCK}.log" + + # Build a log of ~290 KB: OLD_DATA fills the first 160 KB, + # NEW_DATA fills the last 128 KB. Only NEW_DATA should appear in replay. + python3 -c " +import sys +old = b'OLD_DATA_LINE_PADDED_TO_EXACTLY_32B\n' +new = b'NEW_DATA_LINE_PADDED_TO_EXACTLY_32B\n' +old_count = (160 * 1024) // len(old) + 1 +new_count = (128 * 1024) // len(new) + 1 +sys.stdout.buffer.write(old * old_count) +sys.stdout.buffer.write(new * new_count) +" > "$REPLAY_LOG" + + # Use expect to run atch attach with a real PTY, capturing all output. + # atch exits immediately after replaying the log for a dead session. + REPLAY_OUT=$(mktemp) + expect - << EXPECT_EOF > "$REPLAY_OUT" 2>/dev/null +set timeout 10 +spawn $ATCH attach replay-cap-sess +expect eof +EXPECT_EOF + + OUT_BYTES=$(wc -c < "$REPLAY_OUT") + + # Output must stay within SCROLLBACK_SIZE + some terminal-overhead margin + # (expect may inject a few extra bytes; 256 KB is a safe upper bound). + MAX_BYTES=262144 + if [ "$OUT_BYTES" -le "$MAX_BYTES" ]; then + ok "replay-log: output bounded ($OUT_BYTES <= $MAX_BYTES bytes)" + else + fail "replay-log: output bounded" \ + "<= $MAX_BYTES bytes" "$OUT_BYTES bytes" + fi + + # Replayed content must come from the tail (NEW_DATA present). + if grep -q "NEW_DATA" "$REPLAY_OUT" 2>/dev/null; then + ok "replay-log: tail of log replayed (NEW_DATA present)" + else + fail "replay-log: tail of log replayed (NEW_DATA present)" \ + "NEW_DATA in output" "not found" + fi + + # HEAD of log must NOT appear (OLD_DATA absent). + if grep -q "OLD_DATA" "$REPLAY_OUT" 2>/dev/null; then + fail "replay-log: head of log skipped (OLD_DATA absent)" \ + "no OLD_DATA" "OLD_DATA found" + else + ok "replay-log: head of log skipped (OLD_DATA absent)" + fi + + rm -f "$REPLAY_OUT" "$REPLAY_LOG" +else + ok "replay-log: skip (expect or python3 not available)" + ok "replay-log: skip (expect or python3 not available)" + ok "replay-log: skip (expect or python3 not available)" +fi + # ── 22. no-args → usage ────────────────────────────────────────────────────── # Invoking with zero arguments calls usage() (exits 0, prints help). From c2f263e870477820ac354b4090c6f90a5a85bc04 Mon Sep 17 00:00:00 2001 From: Donaldo Date: Tue, 10 Mar 2026 15:17:05 +0100 Subject: [PATCH 14/22] fix(attach): replace ATCH_SESSION-only self-attach guard with PID ancestry check The anti-recursion guard in attach_main relied solely on ATCH_SESSION to detect self-attach attempts. Because ATCH_SESSION is an inherited env var it could be stale (e.g. the variable set inside a session that has since been detached-from, or a shell-config export), causing every subsequent attach to be incorrectly blocked with "cannot attach from within itself". Fix: the master now writes the pty-child PID to .ppid on session start and removes it on cleanup. attach_main reads this file and walks the process-ancestry tree (via libproc on macOS, /proc on Linux) to verify that the calling process is genuinely a descendant of the session shell before blocking the attach. If the .ppid file is absent or the recorded PID is not an ancestor, ATCH_SESSION is treated as stale and the attach proceeds. The ancestry check is also hoisted before require_tty() in cmd_attach and the legacy -a/-A/-c routes so the correct diagnostic fires even without a controlling terminal. Co-Authored-By: Claude Sonnet 4.6 --- atch.c | 8 +++ atch.h | 1 + attach.c | 184 ++++++++++++++++++++++++++++++++++++++++++-------- master.c | 27 ++++++++ tests/test.sh | 65 ++++++++++++++++++ 5 files changed, 257 insertions(+), 28 deletions(-) diff --git a/atch.c b/atch.c index ff8f2f1..04b825b 100644 --- a/atch.c +++ b/atch.c @@ -407,6 +407,9 @@ static int cmd_attach(int argc, char **argv) printf("Try '%s --help' for more information.\n", progname); return 1; } + /* Check ancestry before TTY so the correct error is shown first. */ + if (check_attach_ancestry()) + return 1; save_term(); if (require_tty()) return 1; @@ -835,6 +838,11 @@ int main(int argc, char **argv) return 1; if (mode != 'a') argv = use_shell_if_no_cmd(argc, argv); + /* Check ancestry before TTY so the correct error fires first. */ + if (mode == 'a' || mode == 'A' || mode == 'c') { + if (check_attach_ancestry()) + return 1; + } save_term(); if (dont_have_tty && mode != 'n' && mode != 'N') { printf("%s: attaching to a session requires a " diff --git a/atch.h b/atch.h index 0b59f48..f44e31e 100644 --- a/atch.h +++ b/atch.h @@ -136,6 +136,7 @@ void get_session_dir(char *buf, size_t size); int socket_with_chdir(char *path, int (*fn)(char *)); int replay_session_log(int saved_errno); +int check_attach_ancestry(void); int attach_main(int noerror); int master_main(char **argv, int waitattach, int dontfork); int push_main(void); diff --git a/attach.c b/attach.c index 22006ff..37939c4 100644 --- a/attach.c +++ b/attach.c @@ -8,6 +8,104 @@ #endif #endif +/* ── ancestry helpers ────────────────────────────────────────────────────── */ + +/* +** Return the parent PID of 'pid'. +** Returns 0 on failure (pid not found or permission denied). +** Portable across Linux (/proc) and macOS (libproc / sysctl). +*/ +#ifdef __APPLE__ +#include +static pid_t get_parent_pid(pid_t pid) +{ + struct proc_bsdinfo info; + + if (proc_pidinfo(pid, PROC_PIDTBSDINFO, 0, + &info, sizeof(info)) <= 0) + return 0; + return (pid_t)info.pbi_ppid; +} +#else +static pid_t get_parent_pid(pid_t pid) +{ + char path[64]; + FILE *f; + pid_t ppid = 0; + char line[256]; + + snprintf(path, sizeof(path), "/proc/%d/status", (int)pid); + f = fopen(path, "r"); + if (!f) + return 0; + while (fgets(line, sizeof(line), f)) { + if (sscanf(line, "PPid: %d", &ppid) == 1) + break; + } + fclose(f); + return ppid; +} +#endif + +/* +** Return 1 if 'ancestor_pid' is equal to, or an ancestor of, 'child_pid'. +** Walks the process tree upward; gives up after 1024 steps to avoid loops. +*/ +static int is_ancestor(pid_t ancestor_pid, pid_t child_pid) +{ + pid_t p = child_pid; + int steps = 0; + + while (p > 1 && steps < 1024) { + if (p == ancestor_pid) + return 1; + p = get_parent_pid(p); + steps++; + } + /* Also check the final value (handles the p == ancestor_pid == 1 edge) */ + return (p == ancestor_pid); +} + +/* +** Read the session shell PID from '.ppid'. +** Returns 0 if the file does not exist or cannot be read. +*/ +static pid_t read_session_ppid(const char *sockpath) +{ + char ppid_path[600]; + FILE *f; + long pid = 0; + + snprintf(ppid_path, sizeof(ppid_path), "%s.ppid", sockpath); + f = fopen(ppid_path, "r"); + if (!f) + return 0; + if (fscanf(f, "%ld", &pid) != 1) + pid = 0; + fclose(f); + return (pid > 0) ? (pid_t)pid : 0; +} + +/* +** Return 1 if the current process is genuinely running inside the session +** whose socket path is 'sockpath'. +** +** The check reads '.ppid' (written by the master when it forks +** the pty child) and tests whether that PID is an ancestor of the calling +** process. If the file is absent or the PID is no longer an ancestor, +** the ATCH_SESSION variable is considered stale and the guard is skipped. +*/ +static int session_is_ancestor(const char *sockpath) +{ + pid_t shell_pid = read_session_ppid(sockpath); + + if (shell_pid <= 0) + return 0; /* no .ppid file → assume stale */ + return is_ancestor(shell_pid, getpid()); +} + +/* ─────────────────────────────────────────────────────────────────────────── */ + /* ** The current terminal settings. After coming back from a suspend, we ** restore this. @@ -280,6 +378,57 @@ int replay_session_log(int saved_errno) return 1; } +/* +** Check whether attaching to 'sockname' would be a self-attach (i.e. the +** current process is running inside that session's ancestry chain). +** +** Returns 1 and prints an error if a genuine self-attach is detected. +** Returns 0 if the attach may proceed. +** +** Called before require_tty() so that the correct diagnostic is shown even +** when there is no terminal available. +*/ +int check_attach_ancestry(void) +{ + const char *tosearch = getenv(SESSION_ENVVAR); + + if (!tosearch || !*tosearch) + return 0; + + { + size_t slen = strlen(sockname); + const char *p = tosearch; + + while (*p) { + const char *colon = strchr(p, ':'); + size_t tlen = + colon ? (size_t)(colon - p) : strlen(p); + + if (tlen == slen + && strncmp(p, sockname, tlen) == 0) { + /* Verify we are genuinely inside this + * session before blocking the attach. + * session_is_ancestor() reads the .ppid + * file written by the master and checks + * the process ancestry; if the file is + * absent or the PID is not an ancestor, + * ATCH_SESSION is stale → allow attach. */ + if (session_is_ancestor(sockname)) { + printf + ("%s: cannot attach to session '%s' from within itself\n", + progname, session_shortname()); + return 1; + } + /* Stale ATCH_SESSION — fall through. */ + } + if (!colon) + break; + p = colon + 1; + } + } + return 0; +} + int attach_main(int noerror) { struct packet pkt; @@ -290,34 +439,13 @@ int attach_main(int noerror) /* Refuse to attach to any session in our ancestry chain (catches both * direct self-attach and indirect loops like A -> B -> A). * SESSION_ENVVAR is the colon-separated chain, so scanning it covers - * all ancestors. */ - { - const char *tosearch = getenv(SESSION_ENVVAR); - - if (tosearch && *tosearch) { - size_t slen = strlen(sockname); - const char *p = tosearch; - - while (*p) { - const char *colon = strchr(p, ':'); - size_t tlen = - colon ? (size_t)(colon - p) : strlen(p); - - if (tlen == slen - && strncmp(p, sockname, tlen) == 0) { - if (!noerror) - printf - ("%s: cannot attach to session '%s' from within itself\n", - progname, - session_shortname()); - return 1; - } - if (!colon) - break; - p = colon + 1; - } - } - } + * all ancestors. + * + * The check is performed via check_attach_ancestry(), which is also + * called early in the command handlers (before require_tty) so the + * correct error is shown even without a terminal. */ + if (check_attach_ancestry()) + return 1; /* Attempt to open the socket. Don't display an error if noerror is ** set. */ diff --git a/master.c b/master.c index 9a9ad7d..d9cb3bf 100644 --- a/master.c +++ b/master.c @@ -92,9 +92,28 @@ static int open_log(const char *path) return fd; } +/* Write the pty-child PID to .ppid for ancestry verification. */ +static void write_session_ppid(pid_t pid) +{ + char ppid_path[600]; + int fd; + char buf[32]; + int len; + + snprintf(ppid_path, sizeof(ppid_path), "%s.ppid", sockname); + fd = open(ppid_path, O_WRONLY | O_CREAT | O_TRUNC, 0600); + if (fd < 0) + return; + len = snprintf(buf, sizeof(buf), "%d\n", (int)pid); + write(fd, buf, (size_t)len); + close(fd); +} + /* Write end marker to log, close it, and unlink the socket. */ static void cleanup_session(void) { + char ppid_path[600]; + if (log_fd >= 0) { time_t age = time(NULL) - master_start_time; char agebuf[32]; @@ -109,6 +128,8 @@ static void cleanup_session(void) log_fd = -1; } unlink(sockname); + snprintf(ppid_path, sizeof(ppid_path), "%s.ppid", sockname); + unlink(ppid_path); } /* Signal */ @@ -617,6 +638,12 @@ static void master_process(int s, char **argv, int waitattach, int statusfd) exit(1); } + /* Record the pty-child PID for ancestry verification in attach_main. + * attach_main reads .ppid to confirm that a process trying + * to attach is genuinely running inside this session before blocking + * a re-attach based on a potentially stale ATCH_SESSION value. */ + write_session_ppid(the_pty.pid); + /* Set up some signals. */ signal(SIGPIPE, SIG_IGN); signal(SIGXFSZ, SIG_IGN); diff --git a/tests/test.sh b/tests/test.sh index 52e1003..195fea6 100644 --- a/tests/test.sh +++ b/tests/test.sh @@ -784,6 +784,71 @@ else ok "replay-log: skip (expect or python3 not available)" fi +# ── 21b. ATCH_SESSION ancestry protection ──────────────────────────────────── +# +# Regression test for the ATCH_SESSION stale-ancestry bug. +# +# The anti-recursion guard in attach_main must only fire when the current +# process is genuinely a descendant of the target session. It must NOT fire +# when ATCH_SESSION merely contains the session path but the process is not +# actually running inside that session (stale env var). +# +# Because attach_main is only reached after require_tty() in the normal +# command path, we probe the guard by simulating the session's .ppid file: +# +# • No .ppid file (or PID 0) → guard is bypassed → "does not exist" / "requires a terminal" +# • .ppid file with a PID that IS an ancestor of the current shell → guard fires +# • .ppid file with a PID that is NOT an ancestor (e.g. already-dead PID) → guard bypassed +# +# A session's .ppid file is written by the master and contains the PID of the +# shell process running inside the pty (the_pty.pid). + +mkdir -p "$HOME/.cache/atch" + +# Case A: ATCH_SESSION holds a session path, NO .ppid file exists → no block +GHOST_SOCK="$HOME/.cache/atch/ghost-session" +# No socket, no .ppid — completely absent session +run env ATCH_SESSION="$GHOST_SOCK" "$ATCH" attach ghost-session 2>&1 +assert_exit "ppid-guard: no ppid file → exit 1 (not self-attach)" 1 "$rc" +assert_not_contains "ppid-guard: no ppid file → no self-attach msg" \ + "from within itself" "$out" + +# Case B: .ppid file contains a dead / non-ancestor PID → guard must NOT fire +"$ATCH" start ppid-live sleep 9999 +wait_socket ppid-live +PPID_SOCK="$HOME/.cache/atch/ppid-live" +# Write a PID that is definitely not an ancestor (PID 1 is init/launchd, +# which is NOT a direct ancestor of our test shell in a normal session). +# Using a large unlikely-to-exist PID is fragile; using PID 1 is safe because +# PID 1 is the root, not our direct ancestor in the process hierarchy +# (our shell's ppid is the test runner, not init). +# Actually we need a PID that is NOT in our ancestry. PID of a sleep process works. +DEAD_PID_PROC=$(sh -c 'sleep 60 & echo $!') +sleep 0.05 +kill "$DEAD_PID_PROC" 2>/dev/null +wait "$DEAD_PID_PROC" 2>/dev/null +# DEAD_PID_PROC is now dead — write it as ppid +printf "%d\n" "$DEAD_PID_PROC" > "${PPID_SOCK}.ppid" +run env ATCH_SESSION="$PPID_SOCK" "$ATCH" attach ppid-live 2>&1 +assert_exit "ppid-guard: dead ppid → exit 1 (not self-attach)" 1 "$rc" +assert_not_contains "ppid-guard: dead ppid → no self-attach msg" \ + "from within itself" "$out" +tidy ppid-live + +# Case C: .ppid file contains the PID of our current shell → guard MUST fire +"$ATCH" start self-session sleep 9999 +wait_socket self-session +SELF_SOCK="$HOME/.cache/atch/self-session" +# Write the PID of the current shell ($$) as if this process IS the shell +# running inside the session. From atch's perspective, our process IS a +# descendant of "$$" (itself) — so the guard should trigger. +printf "%d\n" "$$" > "${SELF_SOCK}.ppid" +run env ATCH_SESSION="$SELF_SOCK" "$ATCH" attach self-session 2>&1 +assert_exit "ppid-guard: self as ppid → blocked exit 1" 1 "$rc" +assert_contains "ppid-guard: self as ppid → self-attach msg" \ + "from within itself" "$out" +tidy self-session + # ── 22. no-args → usage ────────────────────────────────────────────────────── # Invoking with zero arguments calls usage() (exits 0, prints help). From 4b6b5270f1a00b73ead9783a2332e7c0772c2b4a Mon Sep 17 00:00:00 2001 From: Ricardo Valfreixo Date: Thu, 12 Mar 2026 12:13:59 +0100 Subject: [PATCH 15/22] Fix log file growing to 2x configured cap log_written tracked bytes since last rotation, not actual file size. After open_log(), the file could already hold log_max_size bytes but log_written started at 0, so another log_max_size was written before rotation triggered. Initialize log_written from the fd position after rotate_log() so the counter reflects reality. --- master.c | 1 + 1 file changed, 1 insertion(+) diff --git a/master.c b/master.c index d9cb3bf..4ca3031 100644 --- a/master.c +++ b/master.c @@ -89,6 +89,7 @@ static int open_log(const char *path) log_fd = fd; rotate_log(); + log_written = (size_t)lseek(log_fd, 0, SEEK_CUR); return fd; } From e8a0c7a0d902724baa8a2871cc2b3fa2d77552eb Mon Sep 17 00:00:00 2001 From: Donaldo Date: Tue, 10 Mar 2026 14:24:17 +0100 Subject: [PATCH 16/22] docs: add fork constitution Documents fork identity, architectural principles (raw PTY passthrough, no emulation, no deps), C style conventions, upstream policy, build instructions, and test process. Co-Authored-By: Claude Sonnet 4.6 --- constitution.md | 145 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 145 insertions(+) create mode 100644 constitution.md diff --git a/constitution.md b/constitution.md new file mode 100644 index 0000000..29f97af --- /dev/null +++ b/constitution.md @@ -0,0 +1,145 @@ +# atch — Fork Constitution + +## 1. Identity + +This repository is a macOS-compatible fork of [mobydeck/atch](https://github.com/mobydeck/atch) +(GPL licence). The upstream project targets Linux exclusively and links with +`-static`; this fork lifts that constraint and adds macOS-specific headers so +the binary builds and runs natively on Darwin. + +The fork exists for two reasons: + +1. **macOS build support** — upstream does not handle `util.h` (Darwin) vs + `pty.h` / `libutil.h` (Linux) and links with `-static` which is unsupported + on macOS. +2. **UX evolutions** — session management improvements that may or may not be + suitable for upstream (see § Upstream policy). + +Fork: +Upstream: + +--- + +## 2. Architectural Principles + +### Raw PTY passthrough — no terminal emulation + +atch multiplexes a PTY session over a Unix socket. The master process owns the +PTY and forwards raw bytes to every attached client; clients write raw bytes back +to the master. There is **no terminal emulation layer**, no VT100/ANSI parser, +no screen buffer reconstruction. Sequences reach the real terminal of each +attaching client unchanged. + +### Minimalism + +- Pure C, no external runtime dependencies beyond the system C library and + `openpty(3)` / `forkpty(3)` (provided by `-lutil` on Linux, `util.h` on + Darwin). +- No autoconf, no cmake, no pkg-config. A single `makefile` drives the build. +- No third-party libraries. If a feature requires a dependency, reconsider the + feature. + +### Source layout + +| File | Role | +|------|------| +| `atch.c` | Main entry point, command dispatch, shared utilities | +| `atch.h` | Shared declarations, includes, protocol constants | +| `config.h` | Compile-time feature flags and tunables | +| `master.c` | PTY master process (session owner) | +| `attach.c` | Attaching client process | + +--- + +## 3. C Style + +Observe and match the conventions already present in the codebase: + +- **Indentation**: tabs (1 tab = 1 level). +- **Brace placement**: opening brace on the same line for control structures; + on a new line for function definitions. +- **Comment style**: `/* single-line */` and the `**`-prefixed block form for + multi-line explanations (`/* \n** text\n*/`). +- **Function length**: keep functions short and focused; extract helpers rather + than nesting logic. +- **Naming**: `snake_case` for functions and variables; `UPPER_CASE` for + macros and `enum` constants. +- **Error handling**: check every syscall return value; use `errno` for + diagnostics; prefer early-return on error over deep nesting. +- **String safety**: `snprintf` instead of `sprintf`; explicit size arguments + on all buffer operations. +- **Compiler warnings**: code must compile cleanly under `-W -Wall`. + +--- + +## 4. Upstream Policy + +| Change type | Action | +|-------------|--------| +| Generic bug fix (Linux + macOS) | Open a PR upstream; cherry-pick the fix here once merged or if upstream is slow to respond | +| macOS-specific fix (e.g. `util.h`, no `-static`) | Keep in this fork; do not send upstream | +| UX feature (session history, log rotation, kill `--force`, …) | Open a PR upstream if the change is general-purpose; keep here otherwise | +| Breaking protocol change | Discuss upstream before implementing | + +The guiding principle: upstream is the source of truth for the protocol and the +core PTY loop. This fork adds a compatibility shim and UX polish; it does not +diverge architecturally. + +--- + +## 5. Build + +### Prerequisites + +- macOS: Xcode Command Line Tools (`xcode-select --install`). +- Linux: `gcc`, `make`, `libutil` (or `libbsd`). + +### Local build + +```sh +make clean && make +``` + +The `makefile` detects the platform via `uname -s` and omits `-static` on +Darwin automatically. + +### Docker / cross-compile (Linux release binary) + +```sh +make build-docker # build Linux binary via Docker +make release # build amd64 + arm64 tarballs in ./release/ +``` + +### Relevant makefile variables + +| Variable | Default | Purpose | +|----------|---------|---------| +| `VERSION` | `dev` | Embedded in the binary via `-DPACKAGE_VERSION` | +| `arch` | host arch | Target architecture for Docker build | +| `BUILDDIR` | `.` | Output directory for the binary | + +--- + +## 6. Tests + +The test suite is a POSIX shell script (`tests/test.sh`) that emits TAP output. +It requires the compiled `atch` binary as its only argument and runs in an +isolated `$HOME` under `/tmp`. + +### Run on Linux (or via Docker) + +```sh +make test # builds Docker image + runs tests inside the container +``` + +### Run directly (if atch is already compiled locally) + +```sh +sh tests/test.sh ./atch +``` + +The tests cover: session create/attach/detach/kill, `push`, `list`, `current`, +`clear`, the `-q` quiet flag, log-cap (`-C`), kill `--force`, and `start`. + +There are currently no unit tests for individual C functions; all tests are +integration tests at the CLI level. From f8450a9b5cb43a2974be08c7da193f91a307faac Mon Sep 17 00:00:00 2001 From: Donaldo Date: Tue, 10 Mar 2026 14:29:43 +0100 Subject: [PATCH 17/22] docs(man): add atch.1 man page (section 1) Documents all commands (attach, new, start, run, push, kill, clear, list, current), all options (-e, -E, -r, -R, -z, -q, -t, -C, -f), FILES, ENVIRONMENT (ATCH_SESSION), EXIT STATUS, and EXAMPLES sections. Includes tests/test_man.sh with 33 TAP assertions verified via mandoc. Also removes the *.1 gitignore rule so that the committed man page is tracked directly rather than generated by pandoc. Co-Authored-By: Claude Sonnet 4.6 --- .gitignore | 1 - atch.1 | 272 ++++++++++++++++++++++++++++++++++++++++++++++ tests/test_man.sh | 102 +++++++++++++++++ 3 files changed, 374 insertions(+), 1 deletion(-) create mode 100644 atch.1 create mode 100644 tests/test_man.sh diff --git a/.gitignore b/.gitignore index 420cca7..159dca2 100644 --- a/.gitignore +++ b/.gitignore @@ -2,7 +2,6 @@ !.gitignore *.o *~ -*.1 *.1.md /atch build/ diff --git a/atch.1 b/atch.1 new file mode 100644 index 0000000..686c7fa --- /dev/null +++ b/atch.1 @@ -0,0 +1,272 @@ +.TH ATCH 1 "2024" "atch" "User Commands" +.SH NAME +atch \- terminal session manager with persistent history and multi-client attach +.SH SYNOPSIS +.B atch +[\fIoptions\fR] [\fIsession\fR [\fIcommand...\fR]] +.br +.B atch +\fIcommand\fR [\fIoptions\fR] ... +.SH DESCRIPTION +.B atch +is a lightweight terminal session manager for macOS (and Linux). +It creates a pseudo-terminal (PTY) managed by a background master process, +allowing multiple clients to attach and detach at will. +Session output is persisted to a disk log so that late-joining clients can +replay what they missed. +.PP +The simplest invocation is: +.PP +.RS +.B atch \fIsession\fR +.RE +.PP +This attaches to an existing session named \fIsession\fR, or creates it (running +your shell) if it does not yet exist. +.PP +A session name without a slash is resolved to a socket under +\fI~/.cache/atch/\fR. A name that contains a slash is used as a full socket +path, which may reside anywhere the current user can write to. +.SH COMMANDS +.TP +.BR "atch " [\fIsession\fR " [" \fIcommand...\fR ]] +Attach-or-create. If \fIsession\fR exists, attach to it. If not, create it +(running \fIcommand\fR, or the user's shell when no command is given) and +attach. Requires a TTY. +.TP +.B atch attach \fIsession\fR +.br +Aliases: \fBa\fR +.br +Strict attach. Fail with exit code 1 if \fIsession\fR does not exist. +Requires a TTY. +.TP +.B atch new \fIsession\fR [\fIcommand...\fR] +.br +Alias: \fBn\fR +.br +Create a new session running \fIcommand\fR (or the user's shell if omitted) and +immediately attach to it. Requires a TTY. +.TP +.B atch start \fIsession\fR [\fIcommand...\fR] +.br +Alias: \fBs\fR +.br +Create a new session in the background (detached). Exits immediately after the +master process is launched. Does not require a TTY. +.TP +.B atch run \fIsession\fR [\fIcommand...\fR] +Create a new session with the master process staying in the foreground +(no fork). Useful for supervisor-managed processes or debugging. +.TP +.B atch push \fIsession\fR +.br +Alias: \fBp\fR +.br +Read from standard input and send the bytes verbatim to \fIsession\fR. +Useful for scripted input injection. Does not require a TTY. +.TP +.B atch kill [\fB\-f\fR|\fB\-\-force\fR] \fIsession\fR +.br +Alias: \fBk\fR +.br +Stop \fIsession\fR by sending SIGTERM to the child process group, waiting for a +short grace period, then sending SIGKILL if the process has not exited. With +\fB\-f\fR or \fB\-\-force\fR, SIGKILL is sent immediately without a grace +period. +.TP +.B atch clear [\fIsession\fR] +Truncate the on-disk log of \fIsession\fR to zero bytes. If \fIsession\fR is +omitted and the environment variable \fBATCH_SESSION\fR is set (i.e. the +command is run from within an atch session), the innermost session in the +ancestry chain is cleared. +.TP +.B atch list +.br +Aliases: \fBl\fR, \fBls\fR +.br +List all sessions in the default session directory. Each entry shows the +session name, age, and whether the socket is alive or stale (\fB[stale]\fR). +.TP +.B atch current +Print the name of the current session (read from \fBATCH_SESSION\fR). When +nested, the full ancestry chain is printed, separated by \fB>\fR. Exits with +code 1 when not inside an atch session. +.SH OPTIONS +The following options may appear before or after the subcommand (except where +noted). Options that take an argument (\fB\-e\fR, \fB\-r\fR, \fB\-R\fR, +\fB\-C\fR) consume the next argument. +.TP +.BI \-e " char" +Set the detach character. \fIchar\fR may be a literal character or a caret +notation such as \fB^A\fR, \fB^B\fR, etc. Use \fB^?\fR for DEL. The default +detach character is \fB^\\\fR (Ctrl-Backslash). +.TP +.B \-E +Disable the detach character entirely. Typing the detach sequence will be +forwarded to the session instead of causing a detach. +.TP +.BI \-r " method" +Set the redraw method used when reattaching. \fImethod\fR is one of: +.RS +.TP +.B none +No automatic redraw. +.TP +.B ctrl_l +Send a Ctrl-L character (form-feed) to the session. +.TP +.B winch +Send a SIGWINCH signal to the session (the default when a TTY is present). +.RE +.TP +.BI \-R " method" +Set the clear method used when reattaching. \fImethod\fR is one of: +.RS +.TP +.B none +No automatic clear (default). +.TP +.B move +Clear by moving the cursor. +.RE +.TP +.B \-z +Disable the suspend key (Ctrl-Z). When set, the suspend character is +forwarded to the session rather than suspending the client. +.TP +.B \-q +Quiet mode. Suppress informational messages such as "session created" or +"session stopped". Error messages are not suppressed. +.TP +.B \-t +Disable VT100/ANSI terminal assumptions. Use this when the local terminal is +not an ANSI-compatible terminal. +.TP +.BI \-C " size" +Set the maximum on-disk log size. Older bytes are trimmed when the log grows +beyond this limit. \fIsize\fR may be a plain integer (bytes), or a number +followed by \fBk\fR/\fBK\fR (kibibytes) or \fBm\fR/\fBM\fR (mebibytes). +Use \fB0\fR to disable logging entirely. The default is \fB1m\fR (1 MiB). +.TP +.B \-f ", " \-\-force +Only valid with the \fBkill\fR subcommand. Skip the SIGTERM grace period and +send SIGKILL immediately. +.SH FILES +.TP +.I ~/.cache/atch/ +Unix domain socket for the named session. +.TP +.I ~/.cache/atch/.log +Persistent output log for the named session. The log is trimmed to the cap +set by \fB\-C\fR (default 1 MiB) every time the limit is reached. When a +session ends, an end marker is appended before the file is closed. +.PP +When \fI$HOME\fR is unset or is the root directory, sockets are stored under +\fI/tmp/.atch-/\fR instead. +.SH ENVIRONMENT +.TP +.B ATCH_SESSION +Set by \fBatch\fR in the environment of every child process it spawns. +Contains the colon-separated ancestry chain of socket paths (outermost first), +ending with the socket of the innermost (current) session. For a +non-nested session, the value is a single socket path with no colon. +.PP +The environment variable name is derived from the basename of the \fBatch\fR +binary at startup: non-alphanumeric characters are replaced with underscores +and the result is uppercased, then \fB_SESSION\fR is appended. For example, +a binary named \fBssh2incus-atch\fR uses \fBSSH2INCUS_ATCH_SESSION\fR. +.TP +.B HOME +Used to locate the default session directory. See \fBFILES\fR above. +.TP +.B SHELL +Used as the default command when no \fIcommand\fR is given to \fBnew\fR, +\fBstart\fR, or the implicit attach-or-create form. Falls back to the passwd +database and then to \fI/bin/sh\fR. +.SH EXIT STATUS +.TP +.B 0 +Success. +.TP +.B 1 +An error occurred (session not found, no TTY available, invalid arguments, etc.) +or the invoked command exited with a non-zero status. +.SH EXAMPLES +Create a new session named \fBwork\fR and attach to it: +.PP +.RS +.B atch work +.RE +.PP +Or equivalently: +.PP +.RS +.B atch new work +.RE +.PP +Detach from the current session by typing the detach sequence \fBCtrl-\\\fR. +.PP +List all sessions: +.PP +.RS +.B atch list +.RE +.PP +Reattach to a running session: +.PP +.RS +.B atch attach work +.RE +.PP +Start a long-running process in the background, without a terminal: +.PP +.RS +.B atch start build make -j8 +.RE +.PP +Inject a command into a running session: +.PP +.RS +.B printf 'echo hello\en' | atch push work +.RE +.PP +Stop a session gracefully: +.PP +.RS +.B atch kill work +.RE +.PP +Stop a session immediately (no grace period): +.PP +.RS +.B atch kill -f work +.RE +.PP +Truncate the session log: +.PP +.RS +.B atch clear work +.RE +.PP +Show the current session name from inside a session: +.PP +.RS +.B atch current +.RE +.PP +Start a session with a custom detach character and 512 KiB log cap: +.PP +.RS +.B atch start -e '^A' -C 512k myapp ./myapp +.RE +.SH SEE ALSO +.BR dtach (1), +.BR tmux (1), +.BR screen (1), +.BR nohup (1) +.SH BUGS +Report bugs at \fIhttps://github.com/mobydeck/atch\fR. +.SH AUTHORS +Originally written by the mobydeck team. +macOS fork maintained at \fIhttps://github.com/mobydeck/atch\fR. diff --git a/tests/test_man.sh b/tests/test_man.sh new file mode 100644 index 0000000..6501629 --- /dev/null +++ b/tests/test_man.sh @@ -0,0 +1,102 @@ +#!/bin/sh +# Man page tests for atch. +# Usage: sh tests/test_man.sh [path-to-atch.1] +# Verifies structure, mandatory sections, and content of the man page. + +MAN_PAGE="${1:-./atch.1}" + +PASS=0 +FAIL=0 +T=0 + +ok() { + T=$((T + 1)); PASS=$((PASS + 1)) + printf "ok %d - %s\n" "$T" "$1" +} + +fail() { + T=$((T + 1)); FAIL=$((FAIL + 1)) + printf "not ok %d - %s\n" "$T" "$1" + [ -n "$2" ] && printf " # expected : %s\n # got : %s\n" "$2" "$3" +} + +assert_contains() { + case "$3" in *"$2"*) ok "$1" ;; *) fail "$1" "(contains '$2')" "$3" ;; esac +} + +assert_exit() { + if [ "$2" = "$3" ]; then ok "$1"; else fail "$1" "exit $2" "exit $3"; fi +} + +printf "TAP version 13\n" + +# ── 1. file exists ──────────────────────────────────────────────────────────── + +if [ -f "$MAN_PAGE" ]; then + ok "man page file exists" +else + fail "man page file exists" "file" "not found at $MAN_PAGE" + printf "\n1..%d\n" "$T" + printf "# %d passed, %d failed\n" "$PASS" "$FAIL" + exit 1 +fi + +CONTENT=$(cat "$MAN_PAGE") + +# ── 2. mandatory roff sections ──────────────────────────────────────────────── + +assert_contains "section NAME present" ".SH NAME" "$CONTENT" +assert_contains "section SYNOPSIS present" ".SH SYNOPSIS" "$CONTENT" +assert_contains "section DESCRIPTION present" ".SH DESCRIPTION" "$CONTENT" +assert_contains "section COMMANDS present" ".SH COMMANDS" "$CONTENT" +assert_contains "section OPTIONS present" ".SH OPTIONS" "$CONTENT" +assert_contains "section FILES present" ".SH FILES" "$CONTENT" +assert_contains "section ENVIRONMENT present" ".SH ENVIRONMENT" "$CONTENT" +assert_contains "section EXIT STATUS present" ".SH EXIT STATUS" "$CONTENT" +assert_contains "section EXAMPLES present" ".SH EXAMPLES" "$CONTENT" +assert_contains "section SEE ALSO present" ".SH SEE ALSO" "$CONTENT" +assert_contains "section AUTHORS present" ".SH AUTHORS" "$CONTENT" + +# ── 3. TH macro (title header) ─────────────────────────────────────────────── + +assert_contains "TH macro section 1" ".TH ATCH 1" "$CONTENT" + +# ── 4. commands documented ─────────────────────────────────────────────────── + +assert_contains "command 'attach' documented" "attach" "$CONTENT" +assert_contains "command 'new' documented" "new" "$CONTENT" +assert_contains "command 'start' documented" "start" "$CONTENT" +assert_contains "command 'run' documented" "run" "$CONTENT" +assert_contains "command 'push' documented" "push" "$CONTENT" +assert_contains "command 'kill' documented" "kill" "$CONTENT" +assert_contains "command 'clear' documented" "clear" "$CONTENT" +assert_contains "command 'list' documented" "list" "$CONTENT" +assert_contains "command 'current' documented" "current" "$CONTENT" + +# ── 5. options documented ───────────────────────────────────────────────────── + +assert_contains "option -e documented" "\\-e" "$CONTENT" +assert_contains "option -E documented" "\\-E" "$CONTENT" +assert_contains "option -r documented" "\\-r" "$CONTENT" +assert_contains "option -R documented" "\\-R" "$CONTENT" +assert_contains "option -z documented" "\\-z" "$CONTENT" +assert_contains "option -q documented" "\\-q" "$CONTENT" +assert_contains "option -t documented" "\\-t" "$CONTENT" +assert_contains "option -C documented" "\\-C" "$CONTENT" +assert_contains "option -f for kill documented" "\\-f" "$CONTENT" + +# ── 6. environment variable documented ─────────────────────────────────────── + +assert_contains "ATCH_SESSION documented" "ATCH_SESSION" "$CONTENT" + +# ── 7. man renders without error ───────────────────────────────────────────── + +mandoc "$MAN_PAGE" > /dev/null 2>&1 +assert_exit "man renders without error (mandoc)" 0 "$?" + +# ── summary ────────────────────────────────────────────────────────────────── + +printf "\n1..%d\n" "$T" +printf "# %d passed, %d failed\n" "$PASS" "$FAIL" + +[ "$FAIL" -eq 0 ] From 00bfd5a6af5f58a595482c2d7fbec3d71ab15a1c Mon Sep 17 00:00:00 2001 From: Donaldo Date: Tue, 10 Mar 2026 14:29:47 +0100 Subject: [PATCH 18/22] build(makefile): add install target and PREFIX variable Adds PREFIX ?= /usr/local and an install target that copies: - the atch binary to $(PREFIX)/bin/atch - the man page to $(PREFIX)/share/man/man1/atch.1 Co-Authored-By: Claude Sonnet 4.6 --- makefile | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/makefile b/makefile index eb43a61..e28b904 100644 --- a/makefile +++ b/makefile @@ -4,6 +4,8 @@ CFLAGS = -g -O2 -W -Wall -I. -DPACKAGE_VERSION=\"$(VERSION)\" LDFLAGS = LIBS = -lutil +PREFIX ?= /usr/local + UNAME_S := $(shell uname -s) ifeq ($(UNAME_S),Darwin) STATIC_FLAG = @@ -31,10 +33,16 @@ atch.1: atch.1.md man: atch.1 +install: atch + install -d $(PREFIX)/bin + install -m 755 atch $(PREFIX)/bin/atch + install -d $(PREFIX)/share/man/man1 + install -m 644 atch.1 $(PREFIX)/share/man/man1/atch.1 + clean: rm -f atch $(OBJ) *.1.md *.c~ -.PHONY: fmt +.PHONY: install fmt fmt: docker run --rm -v "$$PWD":/src -w /src alpine:latest sh -c "apk add --no-cache indent && indent -linux $(SRCS) && indent -linux $(SRCS)" From 329bbefddf7531601529acae74470ac9af388ebb Mon Sep 17 00:00:00 2001 From: Rick Morgans Date: Sun, 15 Mar 2026 11:50:10 +1030 Subject: [PATCH 19/22] test: add fd leak regression test for openpty fallback Cycles 50 sessions under ulimit -n 64 to verify that the openpty fallback properly closes fds on failure paths. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/test.sh | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/test.sh b/tests/test.sh index 0c954c0..400e078 100644 --- a/tests/test.sh +++ b/tests/test.sh @@ -720,6 +720,34 @@ assert_contains "no args: shows Usage:" "Usage:" "$out" run "$ATCH" --help assert_contains "help: shows tail command" "tail" "$out" +# ── 23. fd leak: rapid session cycling under low fd limit ────────────────── +# openpty fallback leaks fds on error paths. Under a tight fd limit, +# leaked fds accumulate and eventually prevent new sessions from starting. + +( + ulimit -n 64 2>/dev/null || true + LEAK_FAIL=0 + i=0 + while [ $i -lt 50 ]; do + out=$("$ATCH" start "leak-$i" sleep 999 2>&1) + lrc=$? + if [ "$lrc" -ne 0 ]; then + LEAK_FAIL=1 + break + fi + "$ATCH" kill "leak-$i" >/dev/null 2>&1 + sleep 0.02 + i=$((i + 1)) + done + i=0; while [ $i -lt 50 ]; do "$ATCH" kill "leak-$i" >/dev/null 2>&1; i=$((i + 1)); done + exit $LEAK_FAIL +) +if [ $? -eq 0 ]; then + ok "fd-leak: 50 create/destroy cycles under ulimit -n 64" +else + fail "fd-leak: session failed under low fd limit (possible fd leak)" "50 cycles" "failed early" +fi + # ── summary ────────────────────────────────────────────────────────────────── printf "\n1..%d\n" "$T" From 6d5568f01616b7f6188fa11a4d532fa861426e5d Mon Sep 17 00:00:00 2001 From: Rick Morgans Date: Sun, 15 Mar 2026 11:50:39 +1030 Subject: [PATCH 20/22] test: add cwd preservation regression test for socket_with_chdir Verifies that a failed socket operation via socket_with_chdir does not leave the process in the wrong working directory. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/test.sh | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test.sh b/tests/test.sh index 0c954c0..0c6fa14 100644 --- a/tests/test.sh +++ b/tests/test.sh @@ -720,6 +720,21 @@ assert_contains "no args: shows Usage:" "Usage:" "$out" run "$ATCH" --help assert_contains "help: shows tail command" "tail" "$out" +# ── 23. cwd preserved after socket failure ───────────────────────────────── +# socket_with_chdir must restore cwd even when the socket operation fails. +# We create the parent dir so chdir succeeds, but the session path is bogus. + +ORIG_PWD=$(pwd) +mkdir -p "$TESTDIR/sockdir" +"$ATCH" kill "$TESTDIR/sockdir/bogus" >/dev/null 2>&1 +AFTER_PWD=$(pwd) +if [ "$ORIG_PWD" = "$AFTER_PWD" ]; then + ok "cwd: preserved after failed socket operation" +else + fail "cwd: preserved after failed socket operation" "$ORIG_PWD" "$AFTER_PWD" + cd "$ORIG_PWD" +fi + # ── summary ────────────────────────────────────────────────────────────────── printf "\n1..%d\n" "$T" From 5cd99171fdf3172c3c69ab3cbfc49c5959257289 Mon Sep 17 00:00:00 2001 From: Rick Morgans Date: Sun, 15 Mar 2026 12:13:37 +1030 Subject: [PATCH 21/22] fix: strict attach does not replay log for dead sessions attach_main() with noerror=0 (strict attach) no longer calls replay_session_log() when the session is not running. It just prints the appropriate error message and exits. The smart-default path (atch ) still replays the log via its own explicit call before creating a new session. Use 'atch tail ' to view logs for dead sessions. Closes #20 Co-Authored-By: Claude Opus 4.6 (1M context) --- attach.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/attach.c b/attach.c index 24f9774..ad60b03 100644 --- a/attach.c +++ b/attach.c @@ -478,23 +478,23 @@ int attach_main(int noerror) const char *name = session_shortname(); if (!noerror) { - if (!replay_session_log(saved_errno)) { - if (saved_errno == ENOENT) - printf - ("%s: session '%s' does not exist\n", - progname, name); - else if (saved_errno == ECONNREFUSED) - printf - ("%s: session '%s' is not running\n", - progname, name); - else if (saved_errno == ENOTSOCK) - printf - ("%s: '%s' is not a valid session\n", - progname, name); - else - printf("%s: %s: %s\n", progname, - sockname, strerror(saved_errno)); - } + /* Strict attach: just print the error, never + * replay the log. Use 'tail' to view logs. */ + if (saved_errno == ENOENT) + printf + ("%s: session '%s' does not exist\n", + progname, name); + else if (saved_errno == ECONNREFUSED) + printf + ("%s: session '%s' is not running\n", + progname, name); + else if (saved_errno == ENOTSOCK) + printf + ("%s: '%s' is not a valid session\n", + progname, name); + else + printf("%s: %s: %s\n", progname, + sockname, strerror(saved_errno)); } return 1; } From e9fc1f25d5b3b6fd58504160820442452f8e6c3e Mon Sep 17 00:00:00 2001 From: Rick Morgans Date: Sun, 15 Mar 2026 12:18:43 +1030 Subject: [PATCH 22/22] test: add regression test for strict attach log replay Verifies that 'atch attach' does not replay the session log when the session has exited. The log content must not appear in the command output. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/test.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test.sh b/tests/test.sh index 0a373d8..e0f321b 100644 --- a/tests/test.sh +++ b/tests/test.sh @@ -1179,6 +1179,19 @@ else cd "$ORIG_PWD" fi +# ── 24. strict attach does not replay log for dead sessions ──────────────── +# atch attach must not dump the log when the session has exited. +# Use 'atch tail' to view logs explicitly. + +rm -f "$HOME/.cache/atch"/*.log 2>/dev/null || true +"$ATCH" start ghost sh -c 'printf "ghost-marker\n"; exit 0' +sleep 0.3 + +run "$ATCH" attach ghost +assert_exit "ghost: strict attach to exited session → exit 1" 1 "$rc" +assert_not_contains "ghost: attach does not replay log" "ghost-marker" "$out" +rm -f "$HOME/.cache/atch/ghost.log" + # ── summary ────────────────────────────────────────────────────────────────── printf "\n1..%d\n" "$T"