fix(session): detach background daemons from launcher groups - #26
Merged
Conversation
A successful stop only accounts for the current generation. Preserve and snapshot all generation records, then verify process ownership before fallback cleanup, including failed restart predecessors.
This was referenced Sep 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Sending SIGHUP to the process group that launched
termctrl startalso kills the background session daemon. The application becomes unreachable even though named sessions are intended to remain available across CLI invocations. Restarted sessions have the same problem because both paths usestart_locked.What Changes
statusfails with connection refusedstatusremains running andshowreturns the application's screenstopremoves the socket and terminates the applicationCall
setsidin the Unix background daemon'spre_exechook, propagating failures through the existing spawn error. The daemon becomes its own session leader before exec.The CLI regressions use an explicitly isolated launcher group, assert it is not the test runner's group, and signal only that owned group. They verify daemon session leadership, status, visible output, restart cleanup, and explicit stop. Cleanup retains every generation's PID record and verifies UID, executable, and the unique fixture argument before fallback signals to individual processes. A third regression deliberately fails the old-generation exit assertion, verifies both generations are cleaned up, and confirms stale PID records do not terminate an independently owned sentinel session.
Scope
Related: #18. Extracts only its background-daemon detachment fix and regression intent; tape playback and pointer automation remain separate. Foreground
run, cursor negotiation, Windows support, and the control protocol are unchanged.Final review covers
4de9fec93aac920e60d355e5f6a8cfad54fba496..5f4f2e3ad4868421a7d92219bf4540c236980a17, rebased ontomainafter #25 merged. The CPR files match that base; the diff contains only the background-daemon hook, integration tests, and Changeset.Includes a patch Changeset for the fixed six-package npm group. No versions, lockfiles, tags, or published artifacts are changed.
Verification
Local validation ran on macOS arm64, with Rust 1.95.0 and Zig 0.15.2. Both Linux CI runs passed before merge, including the new integration regressions.
READY, and stop removes the socket and application.The first full-suite run transiently failed
natural_parent_exit_terminates_pty_holding_descendantsatsrc/session.rs:2297. Its solo rerun, 20 focused repeats, and full-suite retry passed; an independent final solo rerun also passed. This test callsSession::startdirectly, not the named-sessionstart_lockeddaemon-launch path, so it cannot reach the newsetsidhook.finish_exited_outputwaits for PTY EOF rather than disappearance of the orphan descendant PID; the immediatekill(pid, 0)assertion can race orphan reaping. That is a plausible timing explanation, not a confirmed diagnosis of the transient failure.An additional existing release check was run:
It fails locally under Node 26.5.0/npm 11.17.0 because its
npm publish --dry-runrejects the already-published version 1.1.0:You cannot publish over the previously published versions: 1.1.0.The release script and package version are unchanged in this PR. No publish was performed. This is separate from the passing build, test, packed-consumer, and crate checks above.