highperfocused/multica
1
0
Fork 0
mirror of https://github.com/multica-ai/multica.git synced 2026-08-04 07:34:25 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix(agent): reap claude process group on cancellation (#5918) [MUL-5288] (#5957)

Browse Source 
* fix(agent): reap claude process group on cancellation (#5918)

The Claude backend spawned its child with a bare exec.CommandContext, so
cancellation SIGKILLed only the leader. On a resumed stream-json session
(no wall-clock timeout) the MCP servers and tool subprocesses it spawned
were orphaned and kept running — 64+ min in #5918 — while, under
--max-concurrent-tasks 1, holding the only slot and starving the queue.

Put claude in its own process group and drive a group-wide
SIGTERM->grace->SIGKILL on cancel/timeout before closing stdout, mirroring
the fix already made for codex (#4520) and opencode (#4533). Add
claude_cancel_unix_test.go covering the graceful and SIGKILL-escalation
paths.

MUL-5288

Co-authored-by: multica-agent <github@multica.ai>

* fix(agent): gate claude SIGKILL escalation on whole process group

Review found the grace-window escalation keyed off procDone (leader exit),
not the process group. A SIGTERM-ignoring descendant that does not hold
claude's stdout lets the leader exit, closes procDone, and skips the group
SIGKILL — leaking exactly the orphan #5918 targets.

Escalate to a group SIGKILL unless waitProcessGroupGone confirms the whole
group has exited within the grace window (matching codex). It returns as
soon as the group empties, so the graceful path adds no latency. Add a
mixed-signal regression: TERM-respecting leader + TERM-ignoring,
stdio-detached descendant, which fails against the leader-keyed escalation.

MUL-5288

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
Bohan Jiang
2026-07-27 02:01:41 +08:00
committed by GitHub
parent 4931ee6f7a
commit 286ed61bb0
2 changed files with 208 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

65
server/pkg/agent/claude.go
View File

@@ -12,9 +12,23 @@ import (
"path/filepath"
"strings"
"sync"
"sync/atomic"
"syscall"
"time"
)
// claudeTerminateGraceNanos optionally overrides, in nanoseconds, how long a
// cancelled claude process group is given to exit after SIGTERM before it is
// SIGKILLed. Set via atomic store in tests; zero keeps the default.
var claudeTerminateGraceNanos atomic.Int64
func claudeTerminateGrace() time.Duration {
if n := claudeTerminateGraceNanos.Load(); n > 0 {
return time.Duration(n)
}
return 5 * time.Second
}
// claudeBackend implements Backend by spawning the Claude Code CLI
// with --output-format stream-json.
type claudeBackend struct {
@@ -59,6 +73,21 @@ func (b *claudeBackend) Execute(ctx context.Context, prompt string, opts ExecOpt
cmd := exec.CommandContext(runCtx, execPath, args...)
hideAgentWindow(cmd)
// Run claude in its own process group so cancellation can reach the whole
// tree — the claude CLI plus the MCP servers and tool subprocesses it
// spawns — not just the direct child. The default CommandContext behaviour
// SIGKILLs only the leader, which orphans those descendants; on a resumed
// stream-json session with no wall-clock timeout they then keep running and
// burning model budget long after the task was cancelled, and under
// --max-concurrent-tasks 1 starve every queued task (#5918). This mirrors
// the fix already made for codex (#4520) and opencode (#4533).
configureProcessGroup(cmd)
// Take over context cancellation: the default would SIGKILL only the leader
// the instant runCtx is done. We instead drive a graceful group-wide
// SIGTERM→SIGKILL from the cancellation goroutine below and close stdout
// only after the tree has been signalled. Returning nil keeps os/exec from
// racing us with its own kill; WaitDelay remains the hard backstop.
cmd.Cancel = func() error { return nil }
b.cfg.Logger.Info("agent command", "exec", execPath, "args", args)
cmd.WaitDelay = 10 * time.Second
if opts.Cwd != "" {
@@ -104,6 +133,10 @@ func (b *claudeBackend) Execute(ctx context.Context, prompt string, opts ExecOpt
msgCh := make(chan Message, 256)
resCh := make(chan Result, 1)
// procDone closes once cmd.Wait() returns, letting the cancellation handler
// skip a process that already exited and avoid signalling a dead/reused pid.
procDone := make(chan struct{})
// writeClaudeInput runs in its own goroutine so it cannot deadlock
// against the stdout reader. With --verbose --output-format stream-json
// the CLI emits a startup banner before reading its first stdin frame;
@@ -148,10 +181,35 @@ func (b *claudeBackend) Execute(ctx context.Context, prompt string, opts ExecOpt
assistantEventCount := 0
toolUseCount := 0
// Close stdout when the context is cancelled so scanner.Scan() unblocks.
// On cancellation / timeout, terminate claude (and every MCP server and
// tool subprocess it spawned) BEFORE unblocking the scanner. EOF stdin
// to nudge a clean exit, then SIGTERM the whole process group, give it a
// grace period, and SIGKILL the group if any member is still alive.
// SIGKILL is uncatchable, so once delivered no group member can write
// again — only then is it safe to close the stdout read end as a
// last-resort unblock for a scanner a wedged descendant still keeps
// open. WaitDelay is the final backstop (#5918).
go func() {
<-runCtx.Done()
select {
case <-procDone:
return // finished on its own; nothing to terminate
case <-runCtx.Done():
}
closeStdin()
if cmd.Process != nil {
signalProcessGroup(cmd.Process, syscall.SIGTERM)
// Escalate to a group SIGKILL unless the WHOLE process group has
// exited within the grace window. This must key off the process
// group, not procDone: procDone only means cmd.Wait() returned
// for the leader, so a SIGTERM-ignoring descendant that does not
// hold claude's stdout would let the leader exit, close procDone,
// and skip the SIGKILL — leaking exactly the orphan this fix
// targets. waitProcessGroupGone returns as soon as the group is
// empty, so the graceful case adds no latency.
if !waitProcessGroupGone(cmd.Process, claudeTerminateGrace()) {
signalProcessGroup(cmd.Process, syscall.SIGKILL)
}
}
_ = stdout.Close()
}()
@@ -223,8 +281,9 @@ func (b *claudeBackend) Execute(ctx context.Context, prompt string, opts ExecOpt
closeStdin()
// Wait for process exit
// Wait for process exit, then release the cancellation handler.
exitErr := cmd.Wait()
close(procDone)
duration := time.Since(startTime)
// writeDone is buffered (cap 1) and the writer always sends — by the
// time cmd has exited, the prompt write has either succeeded, hit a

146
server/pkg/agent/claude_cancel_unix_test.go Normal file
View File

@@ -0,0 +1,146 @@
//go:build unix
package agent
import (
"context"
"log/slog"
"path/filepath"
"testing"
"time"
)
// claudeCancelFakeScript returns a POSIX-sh script that impersonates a
// long-running `claude` in stream-json mode: it spawns a background grandchild
// (standing in for an MCP server or tool subprocess), records both its own
// (process-group-leader) pid and the grandchild pid, emits a stream-json system
// init line, then streams stdout in a tight loop forever. This is the shape
// that orphaned a descendant for 64+ minutes in #5918 when the daemon only
// killed the leader. When ignoreTerm is true the whole group ignores SIGTERM,
// forcing the SIGKILL escalation path.
func claudeCancelFakeScript(ignoreTerm bool) string {
trap := "trap 'exit 0' TERM\n"
if ignoreTerm {
trap = "trap '' TERM\n"
}
return "#!/bin/sh\n" + trap +
`# Background grandchild so the test can assert the *whole* group is
# terminated on cancellation, not just the direct child.
( sleep 300 ) &
child=$!
if [ -n "$CLAUDE_PID_FILE" ]; then
printf '%s %s\n' "$$" "$child" > "$CLAUDE_PID_FILE"
fi
printf '{"type":"system","session_id":"ses_fake"}\n'
while true; do
printf '{"type":"system","session_id":"ses_fake"}\n'
sleep 0.1
done
`
}
// claudeMixedSignalFakeScript returns a fake `claude` whose leader RESPECTS
// SIGTERM (so it exits and cmd.Wait() returns the instant the group is
// signalled) while a background grandchild IGNORES SIGTERM and detaches its
// stdio, so it holds neither the leader alive nor claude's stdout pipe. This is
// the mixed case that leaks when escalation keys off the leader's exit
// (procDone) instead of the whole process group: the leader is reaped, procDone
// closes, the SIGKILL is skipped, and the SIGTERM-resistant descendant survives
// — the exact #5918 orphan.
func claudeMixedSignalFakeScript() string {
return "#!/bin/sh\n" + "trap 'exit 0' TERM\n" +
`# Grandchild ignores TERM and redirects its stdio away from the pipe so
# it does not keep claude's stdout open after the leader exits.
( trap '' TERM; sleep 300 ) </dev/null >/dev/null 2>&1 &
child=$!
if [ -n "$CLAUDE_PID_FILE" ]; then
printf '%s %s\n' "$$" "$child" > "$CLAUDE_PID_FILE"
fi
printf '{"type":"system","session_id":"ses_fake"}\n'
while true; do
printf '{"type":"system","session_id":"ses_fake"}\n'
sleep 0.1
done
`
}
// TestClaudeCancellationTerminatesProcessGroupGraceful verifies that cancelling
// a run terminates a SIGTERM-respecting claude and its whole process group,
// returns an "aborted" result without hanging, and leaves no orphaned
// descendant.
func TestClaudeCancellationTerminatesProcessGroupGraceful(t *testing.T) {
runClaudeCancellationTest(t, claudeCancelFakeScript(false))
}
// TestClaudeCancellationEscalatesToSIGKILL verifies the worst case from #5918:
// claude (and the descendants it spawned) ignore SIGTERM and keep running.
// Cancellation must escalate to a group SIGKILL, still return promptly, and
// still reap the whole group — without deadlocking on the stdout scanner or
// closing the pipe under a live writer.
func TestClaudeCancellationEscalatesToSIGKILL(t *testing.T) {
claudeTerminateGraceNanos.Store(int64(300 * time.Millisecond))
t.Cleanup(func() { claudeTerminateGraceNanos.Store(0) })
runClaudeCancellationTest(t, claudeCancelFakeScript(true))
}
// TestClaudeCancellationEscalatesWhenDescendantIgnoresTERM is the mixed-signal
// regression for #5918: a SIGTERM-respecting leader plus a SIGTERM-ignoring,
// stdio-detached descendant. Cancellation must still reap the descendant, which
// only holds when the SIGKILL escalation is gated on the whole process group
// (not the leader's exit). This fails against the leader-keyed escalation.
func TestClaudeCancellationEscalatesWhenDescendantIgnoresTERM(t *testing.T) {
claudeTerminateGraceNanos.Store(int64(300 * time.Millisecond))
t.Cleanup(func() { claudeTerminateGraceNanos.Store(0) })
runClaudeCancellationTest(t, claudeMixedSignalFakeScript())
}
func runClaudeCancellationTest(t *testing.T, script string) {
t.Helper()
tempDir := t.TempDir()
pidFile := filepath.Join(tempDir, "pids")
fakePath := filepath.Join(tempDir, "claude")
writeTestExecutable(t, fakePath, []byte(script))
backend, err := New("claude", Config{
ExecutablePath: fakePath,
Logger: slog.Default(),
Env: map[string]string{"CLAUDE_PID_FILE": pidFile},
})
if err != nil {
t.Fatalf("new claude backend: %v", err)
}
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
session, err := backend.Execute(ctx, "prompt-ignored", ExecOptions{Cwd: tempDir})
if err != nil {
t.Fatalf("execute: %v", err)
}
// Drain streamed messages so the reader never blocks on a full channel.
go func() {
for range session.Messages {
}
}()
pids := waitForPids(t, pidFile)
cancel() // user cancels the task
select {
case res := <-session.Result:
if res.Status != "aborted" {
t.Errorf("status = %q, want aborted", res.Status)
}
case <-time.After(10 * time.Second):
t.Fatal("Execute did not return after cancellation (possible scanner deadlock or unkilled process)")
}
// The leader and the grandchild must both be gone — cancellation reaped the
// whole group, leaving no orphan spinning.
for _, pid := range pids {
waitProcessGone(t, pid)
}
}