diff --git a/TODO.md b/TODO.md index e1f93f9..e69de29 100644 --- a/TODO.md +++ b/TODO.md @@ -1,2 +0,0 @@ -- [ ] When the parent agent/orchestrator is killed, all child processes spawned by it should also be killed. - - [ ] This should apply to when we detect the process dies, like if the user Ctrl + C's out. diff --git a/internal/app/session.go b/internal/app/session.go index 31658ce..acb5e5c 100644 --- a/internal/app/session.go +++ b/internal/app/session.go @@ -323,6 +323,54 @@ func (s *Session) reapChild(c *Child) { c.markExited(err) logf("child %s exited (err=%v)", c.ID, err) s.emitExit(c) + s.killDescendantsOf(c.ID) +} + +// killDescendantsOf terminates every still-live direct child of +// parentID. SPEC §2: closing the patterm process tears down every +// child it spawned; the same rule applies in-session — when an +// orchestrator dies (natural exit, user Ctrl-C, MCP close, anything +// that makes its PTY EOF), the agents/commands/terminals it spawned +// must die with it. We only signal direct children here: each +// descendant's own reapChild will fire and recurse, so the cascade +// flows through arbitrary depth without us walking the tree. +func (s *Session) killDescendantsOf(parentID string) { + if parentID == "" { + return + } + s.mu.Lock() + var live []*Child + for _, c := range s.children { + if c.ParentID == parentID && c.IsLive() { + live = append(live, c) + } + } + s.mu.Unlock() + if len(live) == 0 { + return + } + for _, c := range live { + _ = c.signal(syscall.SIGTERM) + } + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + anyLive := false + for _, c := range live { + if c.IsLive() { + anyLive = true + break + } + } + if !anyLive { + return + } + time.Sleep(20 * time.Millisecond) + } + for _, c := range live { + if c.IsLive() { + _ = c.signal(syscall.SIGKILL) + } + } } // Children returns a snapshot of children in spawn order. diff --git a/internal/app/session_test.go b/internal/app/session_test.go new file mode 100644 index 0000000..020ac30 --- /dev/null +++ b/internal/app/session_test.go @@ -0,0 +1,82 @@ +package app + +import ( + "syscall" + "testing" + "time" +) + +// TestParentExitKillsDescendants verifies that when a parent process +// exits, every still-live process that was spawned under it is signaled +// and dies, recursively through the tree. +func TestParentExitKillsDescendants(t *testing.T) { + sess := NewSession(t.TempDir(), "test") + + // Use a tiny pause-then-trap shell so the children survive long + // enough for the parent to die first; SIGTERM should terminate them. + sleepArgv := []string{"sh", "-c", "trap 'exit 0' TERM; sleep 30"} + + parent, err := sess.Spawn(SpawnSpec{ + Kind: KindTerminal, + Argv: []string{"sh", "-c", "sleep 30"}, + }, 80, 24) + if err != nil { + t.Fatalf("spawn parent: %v", err) + } + + childA, err := sess.Spawn(SpawnSpec{ + Kind: KindCommand, + Argv: sleepArgv, + ParentID: parent.ID, + }, 80, 24) + if err != nil { + t.Fatalf("spawn childA: %v", err) + } + grandchild, err := sess.Spawn(SpawnSpec{ + Kind: KindCommand, + Argv: sleepArgv, + ParentID: childA.ID, + }, 80, 24) + if err != nil { + t.Fatalf("spawn grandchild: %v", err) + } + + // Wait for everyone to be running. + waitUntilLive(t, parent) + waitUntilLive(t, childA) + waitUntilLive(t, grandchild) + + // Kill the parent. Its reapChild should cascade to childA, whose + // reapChild should in turn cascade to grandchild. + if err := parent.signal(syscall.SIGTERM); err != nil { + t.Fatalf("signal parent: %v", err) + } + + waitUntilNotLive(t, parent) + waitUntilNotLive(t, childA) + waitUntilNotLive(t, grandchild) +} + +func waitUntilLive(t *testing.T, c *Child) { + t.Helper() + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + if c.IsLive() { + return + } + time.Sleep(20 * time.Millisecond) + } + t.Fatalf("child %s never went live", c.ID) +} + +func waitUntilNotLive(t *testing.T, c *Child) { + t.Helper() + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + if !c.IsLive() { + return + } + time.Sleep(20 * time.Millisecond) + } + t.Fatalf("child %s still live after parent died (status=%s)", c.ID, c.Status()) +}