fix(review,investigate): kill agent process groups so Ctrl+C can't hang · Entire

fix(review,investigate): kill agent process groups so Ctrl+C can't hang

0f184b3→main·

dipree·2w ago·6 files·+130 added/-0 removed

Agent CLIs (codex, claude, ...) spawn helper grandchildren (sandbox, MCP servers) that inherit the agent's stdout pipe. exec.CommandContext's default Cancel only SIGKILLs the agent itself, so on Ctrl+C / reviewer timeout the grandchildren stay alive holding the pipe open. The reviewer template drains Events() (the stdout pipe) to EOF before calling Wait, so the read blocks forever, the per-agent goroutine never finishes, the fan-in channel never closes, and RunMulti hangs — the dashboard sits on "cancelling" indefinitely. Investigate's cmd.Run has the same exposure.

Add procutil.TerminateOnCancel (process-group SIGKILL on cancel + a WaitDelay backstop) and apply it on the agent spawn path in both the review template and the investigate loop. Mirrors the existing checkpoint/remote git-transport fix.

Sessions

15b7eec39aa5View transcript

Changes

6

42 unmodified lines
43
44
45
46
47
48
49
262 unmodified lines
312
313
314
315
316
317
318
319
320

42 unmodified lines

"github.com/entireio/cli/cmd/entire/cli/agent/spawn"
    "github.com/entireio/cli/cmd/entire/cli/logging"
    "github.com/entireio/cli/cmd/entire/cli/procutil"
// LoopDeps collects the runtime-injectable hooks RunInvestigateLoop needs.
262 unmodified lines

StartingSHA: in.StartingSHA,
    })
    cmd := spawner.BuildCmd(ctx, env, prompt)
    // Kill the agent's whole process group on cancel so a grandchild holding the
    // pipe can't make cmd.Run block forever (Ctrl+C hang).
    procutil.TerminateOnCancel(cmd)

// Agent stdout/stderr are captured by the lifecycle hooks into the
    // session transcript (full.jsonl) and condensed onto

// Package procutil holds helpers for terminating spawned subprocesses and
// their descendants when a context is cancelled.
package procutil

import (
    "os/exec"
    "time"
)

// terminateWaitDelay backstops the wait after ctx-cancel so Wait/Run returns
// even if a wedged descendant keeps an output pipe open after the group kill.
const terminateWaitDelay = 5 * time.Second

// TerminateOnCancel makes cmd and its descendants die when cmd's context is
// cancelled, and guarantees Wait/Run returns even if a descendant keeps an
// output pipe open. Call after building cmd, before Start/Run.
//
// Agent CLIs (codex, claude, ...) spawn helper grandchildren (sandbox, MCP
// servers) that inherit the stdout pipe. exec.CommandContext's default Cancel
// only SIGKILLs the agent itself, leaving grandchildren alive with the pipe
// open — so a reader draining stdout to EOF blocks forever and Ctrl+C hangs.
func TerminateOnCancel(cmd *exec.Cmd) {
    cmd.WaitDelay = terminateWaitDelay
    killProcessGroupOnCancel(cmd)
}

//go:build unix

package procutil

import (
    "fmt"
    "os/exec"
    "syscall"
)

// killProcessGroupOnCancel SIGKILLs the whole process group on ctx-cancel, so
// grandchildren that inherited the output pipe die too.
func killProcessGroupOnCancel(cmd *exec.Cmd) {
    if cmd.SysProcAttr == nil {
        cmd.SysProcAttr = &syscall.SysProcAttr{}
    }
    cmd.SysProcAttr.Setpgid = true
    cmd.Cancel = func() error {
        if cmd.Process == nil {
            return nil
        }
        // Negative PID = whole group (leader pid == pgid). ESRCH = already exited.
        if err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL); err != nil && err != syscall.ESRCH {
            return fmt.Errorf("kill process group: %w", err)
        }
        return nil
    }
}

//go:build unix

package procutil

import (
    "bufio"
    "context"
    "os/exec"
    "testing"
    "time"
)

// A grandchild that inherits stdout keeps the pipe open after the parent exits.
// Without TerminateOnCancel, draining stdout to EOF blocks until the grandchild
// dies on its own (here ~60s). The group-kill on cancel must unblock it fast.
func TestTerminateOnCancel_UnblocksGrandchildHoldingPipe(t *testing.T) {
    if _, err := exec.LookPath("sh"); err != nil {
        t.Skip("sh not available")
    }

ctx, cancel := context.WithCancel(context.Background())
    defer cancel()

// Parent prints "ready", then exits, leaving a backgrounded sleep that
    // inherited the stdout pipe.
    cmd := exec.CommandContext(ctx, "sh", "-c", "sleep 60 & echo ready")
    TerminateOnCancel(cmd)

stdout, err := cmd.StdoutPipe()
    if err != nil {
        t.Fatalf("stdout pipe: %v", err)
    }
    if err := cmd.Start(); err != nil {
        t.Fatalf("start: %v", err)
    }

r := bufio.NewReader(stdout)
    if _, err := r.ReadString('\n'); err != nil {
        t.Fatalf("read ready line: %v", err)
    }

// Drain to EOF in the background, mirroring how the reviewer reads Events.

drained := make(chan struct{})
go func() {
    _, _ = r.ReadString('\n') // blocks on the open pipe until cancel closes it
    close(drained)
}()

cancel()

select {
    case <-drained:
    case <-time.After(15 * time.Second):
        t.Fatal("stdout drain did not unblock after cancel — Ctrl+C would hang")
    }

_ = cmd.Wait()
}

//go:build windows

package procutil

import "os/exec"

// killProcessGroupOnCancel is a no-op on Windows: reliable tree-kill needs a Job
// Object. The WaitDelay backstop still bounds the wait on a hung subprocess.
func killProcessGroupOnCancel(_ *exec.Cmd) {}