Skip to content

fix: strict attach does not replay log for dead sessions#26

Open
rmorgans wants to merge 28 commits intomobydeck:mainfrom
rmorgans:fix/strict-attach-no-replay
Open

fix: strict attach does not replay log for dead sessions#26
rmorgans wants to merge 28 commits intomobydeck:mainfrom
rmorgans:fix/strict-attach-no-replay

Conversation

@rmorgans
Copy link

@rmorgans rmorgans commented Mar 15, 2026

Summary

  • atch attach <session> no longer replays the session log when the session is dead. It prints the appropriate error message ("does not exist" / "is not running") and exits with code 1, matching the documented contract ("strict attach: fail if session missing").
  • The smart-default path (atch <session>) still replays the log before creating a new session — that behavior is unchanged.
  • Use atch tail <session> to explicitly view logs for dead sessions.

Closes #20

Test plan

  • atch start foo sleep 5 && sleep 6 && atch attach foo → prints error, no log replay
  • atch foo (smart default, dead session with log) → still replays log before creating
  • atch tail foo → shows log contents as expected
  • Builds clean on macOS

🤖 Generated with Claude Code

rmorgans and others added 27 commits March 14, 2026 22:48
@rmorgans @claude
fix: close leaked fds in openpty fallback, add static assertions
988d54b 
- 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) <noreply@anthropic.com>
@rmorgans @claude
fix: defensive checks for cwd restore, malloc, and log writes
5dbb1f6 
- 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) <noreply@anthropic.com>
@rmorgans @claude
fix: retry partial writes in packet and push paths
c6a9d18 
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) <noreply@anthropic.com>
@rmorgans @claude
fix: use write_all in send_kill (missed in initial fix)
851934a 
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) <noreply@anthropic.com>
@rmorgans @claude
fix: treat write()=0 as failure, add regression tests
6b8c395 
- 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) <noreply@anthropic.com>
@rmorgans @claude
fix: reject SCROLLBACK_SIZE == 0 in static assertion
adc1181 
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) <noreply@anthropic.com>
@rmorgans @claude
fix: guard all log operations behind log_fd >= 0
4d82034 
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) <noreply@anthropic.com>
@rmorgans @claude
fix: make signal handlers async-signal-safe and handle blocked writes
0f0f734 
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) <noreply@anthropic.com>
@rmorgans @claude
test: add forkpty-based signal safety integration tests
fea3204 
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) <noreply@anthropic.com>
@rmorgans @claude
chore: ignore *.dSYM directories in .gitignore
92a9532 
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@DonaldoDes @claude @rmorgans
fix(master): use umask(0177) in create_socket to prevent S_IXUSR race
b9f3aa3 
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 <noreply@anthropic.com>
@DonaldoDes @claude @rmorgans
fix(attach): send MSG_DETACH before exit on detach_char press
31769a0 
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 <noreply@anthropic.com>
@DonaldoDes @claude @rmorgans
fix(attach): cap session log replay to last 128KB
dbd04ec 
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 <noreply@anthropic.com>
@DonaldoDes @claude @rmorgans
fix(attach): replace ATCH_SESSION-only self-attach guard with PID anc…
c2f263e 
…estry 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 <sockname>.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 <noreply@anthropic.com>
@ricardovfreixo @rmorgans
Fix log file growing to 2x configured cap
4b6b527 
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.
@DonaldoDes @claude @rmorgans
docs: add fork constitution
e8a0c7a 
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 <noreply@anthropic.com>
@DonaldoDes @claude @rmorgans
docs(man): add atch.1 man page (section 1)
f8450a9 
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 <noreply@anthropic.com>
@DonaldoDes @claude @rmorgans
build(makefile): add install target and PREFIX variable
00bfd5a 
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 <noreply@anthropic.com>
@rmorgans
Merge fix/write-all (mobydeck#22): retry partial writes in packet and…
b2ebbe3 
… push paths

# Conflicts:
#	tests/test.sh
@rmorgans
Merge fix/signal-safety (mobydeck#25): async-signal-safe handlers and…
ec873ff 
… blocked-write fix
@rmorgans
Merge fix/defensive-checks (mobydeck#23): guard cwd restore, malloc, …
fbe3b3f 
…and log writes

# Conflicts:
#	master.c
@rmorgans
Merge fix/openpty-and-asserts (mobydeck#24): close leaked fds, add st…
f83835e 
…atic assertions
@rmorgans @claude
test: add fd leak regression test for openpty fallback
329bbef 
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) <noreply@anthropic.com>
@rmorgans @claude
test: add cwd preservation regression test for socket_with_chdir
6d5568f 
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) <noreply@anthropic.com>
@rmorgans
Update fix/defensive-checks: add cwd preservation regression test
0b1e8ba 
# Conflicts:
#	tests/test.sh
@rmorgans
Update fix/openpty-and-asserts: add fd leak regression test
f1aa8bb 
# Conflicts:
#	tests/test.sh
@rmorgans @claude
fix: strict attach does not replay log for dead sessions
5cd9917 
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 <session>) still replays the log
via its own explicit call before creating a new session.

Use 'atch tail <session>' to view logs for dead sessions.

Closes mobydeck#20

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Mar 15, 2026
Open
Open
@rmorgans @claude
test: add regression test for strict attach log replay
e9fc1f2 
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) <noreply@anthropic.com>
rmorgans added a commit to rmorgans/atch that referenced this pull request Mar 15, 2026
@rmorgans
Merge fix/strict-attach-no-replay (mobydeck#26): strict attach does n…
0b002bd 
…ot replay log

# Conflicts:
#	tests/test.sh
rmorgans added a commit to rmorgans/atch that referenced this pull request Mar 15, 2026
@rmorgans @claude
fix: repair two test failures for CI
b16f25a 
replay-log: use smart-open path instead of strict attach, which no
longer replays dead-session logs after PR mobydeck#26.

dash-binary env var: resolve symlink source to absolute path (fixes
macOS), replace fixed sleep with poll loop (fixes CI timing).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] attach to a non-existent session prints log if it is present

3 participants

@rmorgans @DonaldoDes @ricardovfreixo