fix: prevent leaked agent processes on shutdown (#1498)

This commit is contained in:
Carlos Florêncio
2026-06-26 10:09:23 +01:00
committed by GitHub
parent b3e8af5519
commit 49c482bd4d
73 changed files with 3976 additions and 293 deletions
+1 -1
View File
@@ -214,7 +214,7 @@ start:
@$(MAKE) -s build-backend-quiet
$(call success,Build complete)
$(call phase,Starting Server)
@$(BACKEND_DIR)/bin/kandev start $(if $(filter 1 true yes,$(VERBOSE)),--verbose,) $(if $(filter 1 true yes,$(DEBUG)),--debug,)
@exec $(BACKEND_DIR)/bin/kandev start $(if $(filter 1 true yes,$(VERBOSE)),--verbose,) $(if $(filter 1 true yes,$(DEBUG)),--debug,)
.PHONY: start-verbose
start-verbose:
+5
View File
@@ -148,6 +148,11 @@ apps/backend/
- Exposes workspace operations (shell, git, files)
- Supports multiple concurrent instances on different ports
Standalone agentctl is launched in its own process group so terminal Ctrl+C is
handled by the backend lifecycle manager first. Do not make standalone agentctl
share the backend's foreground process group; that bypasses supervised shutdown
and can leak ACP subprocesses.
**Executor Types** (database model):
- `local_pc` - Standalone process on host
- `local_docker` - Docker container on host
+8 -53
View File
@@ -21,9 +21,8 @@ import (
"os"
"os/exec"
"os/signal"
"unsafe"
"golang.org/x/sys/windows"
"github.com/kandev/kandev/internal/agentctl/server/winproc"
)
func main() {
@@ -47,33 +46,27 @@ func main() {
}
}()
job, err := createKillOnCloseJob()
if err != nil {
die("create job: %v", err)
}
// Don't `defer CloseHandle(job)` — we want the OS to close it as part of
// process teardown so KILL_ON_JOB_CLOSE fires correctly on any exit path
// including os.Exit and panics. Manual close on success is also fine
// because the child has already exited by then.
cmd := exec.Command(os.Args[1], os.Args[2:]...)
cmd.Stdin = os.Stdin
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
cmd.Env = os.Environ()
if err := cmd.Start(); err != nil {
_ = windows.CloseHandle(job)
die("start child: %v", err)
}
if err := assignProcessToJob(job, cmd.Process.Pid); err != nil {
job, err := winproc.InstallKillOnCloseJobForCommand(cmd)
if err != nil {
_ = cmd.Process.Kill()
_ = windows.CloseHandle(job)
die("assign job: %v", err)
}
// Don't `defer job.Close()` — we want the OS to close it as part of
// process teardown so KILL_ON_JOB_CLOSE fires correctly on any exit path
// including os.Exit and panics. Manual close on success is also fine
// because the child has already exited by then.
waitErr := cmd.Wait()
_ = windows.CloseHandle(job)
_ = job.Close()
if waitErr != nil {
if exitErr, ok := waitErr.(*exec.ExitError); ok {
os.Exit(exitErr.ExitCode())
@@ -82,44 +75,6 @@ func main() {
}
}
func createKillOnCloseJob() (windows.Handle, error) {
job, err := windows.CreateJobObject(nil, nil)
if err != nil {
return 0, fmt.Errorf("CreateJobObject: %w", err)
}
info := windows.JOBOBJECT_EXTENDED_LIMIT_INFORMATION{
BasicLimitInformation: windows.JOBOBJECT_BASIC_LIMIT_INFORMATION{
LimitFlags: windows.JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE,
},
}
if _, err := windows.SetInformationJobObject(
job,
windows.JobObjectExtendedLimitInformation,
uintptr(unsafe.Pointer(&info)),
uint32(unsafe.Sizeof(info)),
); err != nil {
_ = windows.CloseHandle(job)
return 0, fmt.Errorf("SetInformationJobObject: %w", err)
}
return job, nil
}
func assignProcessToJob(job windows.Handle, pid int) error {
procHandle, err := windows.OpenProcess(
windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE,
false,
uint32(pid),
)
if err != nil {
return fmt.Errorf("OpenProcess(%d): %w", pid, err)
}
defer windows.CloseHandle(procHandle)
if err := windows.AssignProcessToJobObject(job, procHandle); err != nil {
return fmt.Errorf("AssignProcessToJobObject: %w", err)
}
return nil
}
func die(format string, args ...any) {
fmt.Fprintf(os.Stderr, "winjob: "+format+"\n", args...)
os.Exit(1)
+5 -5
View File
@@ -208,11 +208,11 @@ type RuntimeConfig struct {
// agents whose ACP adapter does not wire session/new mcpServers through to
// the underlying CLI.
ProjectMCPStrategy mcpconfig.PassthroughMCPStrategy
// RequiresProcessKill is true for agents whose subprocess does not exit
// when stdin is closed (e.g. OpenCode's ACP runtime, which keeps its HTTP
// server and MCP child tree alive). When true, the agentctl process
// manager kills the whole process group on shutdown so MCP children
// don't leak.
// RequiresProcessKill is true for agents whose subprocess should skip the
// graceful stdin-close wait because it is known not to exit on EOF (e.g.
// OpenCode's ACP runtime, which keeps its HTTP server and MCP child tree
// alive). Agentctl still reaps the process group for every agent after
// graceful shutdown or timeout; this flag only makes that reap immediate.
RequiresProcessKill bool
}
@@ -98,8 +98,8 @@ func (a *OpenCodeACP) Runtime() *RuntimeConfig {
ProjectSkillDir: ".agents/skills",
UserSkillDir: ".config/opencode/skills",
// opencode acp runs its HTTP server + MCP child tree alongside the
// ACP stdin/stdout. Closing stdin doesn't terminate the process —
// we have to kill its process group to reap the MCP children.
// ACP stdin/stdout. Closing stdin doesn't terminate the process, so
// skip the graceful wait and reap its process group immediately.
// See GH issue #1247.
RequiresProcessKill: true,
SessionConfig: SessionConfig{
@@ -8,9 +8,9 @@ import (
// TestOpenCodeACPRuntime_RequiresProcessKill is the regression test for GH
// issue #1247: opencode acp keeps its HTTP server + MCP child tree alive
// when stdin closes, so its RuntimeConfig must signal that the process
// group has to be killed on shutdown. Without this flag the ACP adapter
// returns RequiresProcessKill=false and the process manager only closes
// stdin — opencode never exits and MCP children leak.
// group should be reaped immediately. Without this flag the ACP adapter
// returns RequiresProcessKill=false and the process manager waits for the
// graceful EOF path before it falls back to process-group cleanup.
func TestOpenCodeACPRuntime_RequiresProcessKill(t *testing.T) {
rt := NewOpenCodeACP().Runtime()
if rt == nil {
@@ -23,9 +23,8 @@ func TestOpenCodeACPRuntime_RequiresProcessKill(t *testing.T) {
// TestACPAgents_DefaultProcessKill confirms the rest of the ACP agents
// stick with the default (false). They communicate over plain stdin/stdout
// and exit on EOF, so the process manager's graceful-stdin-close path is
// the correct shutdown signal — flipping this true would force-kill agents
// that are perfectly capable of cleanup.
// and should get a short graceful EOF path before the process manager reaps
// any remaining process-group descendants.
func TestACPAgents_DefaultProcessKill(t *testing.T) {
cases := []struct {
name string
@@ -48,6 +48,8 @@ type Manager struct {
mu sync.RWMutex
instances map[string]*instance // keyed by agent type
createGroup singleflight.Group
startCancel context.CancelFunc
stopped bool
}
// instance is a single warm agentctl instance bound to an agent type.
@@ -86,13 +88,39 @@ func (m *Manager) SetAuthToken(token string) {
// initial probe against each in parallel. Individual agent failures are
// captured in the cache but do not abort the other agents.
func (m *Manager) Start(ctx context.Context) error {
startCtx, cancel := context.WithCancel(ctx)
m.mu.Lock()
if m.stopped {
m.mu.Unlock()
cancel()
return nil
}
m.startCancel = cancel
m.mu.Unlock()
defer func() {
m.mu.Lock()
m.startCancel = nil
m.mu.Unlock()
cancel()
}()
// Create a process-scoped parent dir so concurrent kandev processes do not
// share state, and so Stop only removes dirs owned by this process.
parent, err := os.MkdirTemp("", fmt.Sprintf("kandev-host-utility-%d-*", os.Getpid()))
if err != nil {
return fmt.Errorf("create host utility tmp dir: %w", err)
}
m.mu.Lock()
if m.stopped {
m.mu.Unlock()
if err := os.RemoveAll(parent); err != nil {
m.log.Warn("failed to remove unused host utility parent tmp dir",
zap.String("path", parent), zap.Error(err))
}
return nil
}
m.parentTmpDir = parent
m.mu.Unlock()
m.log.Info("host utility parent tmp dir created", zap.String("path", parent))
targets := m.eligibleAgents()
@@ -101,7 +129,7 @@ func (m *Manager) Start(ctx context.Context) error {
return nil
}
g, gctx := errgroup.WithContext(ctx)
g, gctx := errgroup.WithContext(startCtx)
for _, ag := range targets {
ag := ag
g.Go(func() error {
@@ -118,14 +146,47 @@ func (m *Manager) Start(ctx context.Context) error {
// are untouched.
func (m *Manager) Stop(ctx context.Context) {
m.mu.Lock()
m.stopped = true
cancel := m.startCancel
m.startCancel = nil
instances := make([]*instance, 0, len(m.instances))
for _, inst := range m.instances {
instances = append(instances, inst)
}
m.instances = make(map[string]*instance)
parentTmpDir := m.parentTmpDir
m.parentTmpDir = ""
m.mu.Unlock()
if cancel != nil {
cancel()
}
for _, inst := range instances {
deleteCtx, cancel := hostUtilityDeleteContext(ctx)
m.deleteInstance(deleteCtx, inst)
cancel()
}
if parentTmpDir != "" {
if err := os.RemoveAll(parentTmpDir); err != nil {
m.log.Warn("failed to remove host utility parent tmp dir",
zap.String("path", parentTmpDir), zap.Error(err))
}
}
}
const hostUtilityDeleteTimeout = 2 * time.Second
func hostUtilityDeleteContext(parent context.Context) (context.Context, context.CancelFunc) {
return context.WithTimeout(context.WithoutCancel(parent), hostUtilityDeleteTimeout)
}
func (m *Manager) deleteInstance(ctx context.Context, inst *instance) {
if inst == nil {
return
}
if m.controlClient != nil {
if err := m.controlClient.DeleteInstance(ctx, inst.instanceID); err != nil {
m.log.Warn("failed to delete host utility instance",
zap.String("agent_type", inst.agentType),
@@ -133,13 +194,14 @@ func (m *Manager) Stop(ctx context.Context) {
zap.Error(err))
}
}
if m.parentTmpDir != "" {
if err := os.RemoveAll(m.parentTmpDir); err != nil {
m.log.Warn("failed to remove host utility parent tmp dir",
zap.String("path", m.parentTmpDir), zap.Error(err))
}
m.parentTmpDir = ""
if inst.workDir == "" {
return
}
if err := os.RemoveAll(inst.workDir); err != nil {
m.log.Warn("failed to remove host utility work dir",
zap.String("agent_type", inst.agentType),
zap.String("path", inst.workDir),
zap.Error(err))
}
}
@@ -217,7 +279,21 @@ func (m *Manager) bootstrapAgent(ctx context.Context, ia agents.InferenceAgent)
return
}
if ctx.Err() != nil {
deleteCtx, cancel := hostUtilityDeleteContext(ctx)
m.deleteInstance(deleteCtx, inst)
cancel()
return
}
m.mu.Lock()
if m.stopped {
m.mu.Unlock()
deleteCtx, cancel := hostUtilityDeleteContext(ctx)
m.deleteInstance(deleteCtx, inst)
cancel()
return
}
m.instances[agentType] = inst
m.mu.Unlock()
@@ -242,7 +318,17 @@ func (m *Manager) createInstance(ctx context.Context, agentType string) (*instan
if !safeAgentTypeName.MatchString(agentType) {
return nil, fmt.Errorf("invalid agent type %q: must match %s", agentType, safeAgentTypeName.String())
}
workDir := filepath.Join(m.parentTmpDir, agentType)
m.mu.RLock()
parentTmpDir := m.parentTmpDir
stopped := m.stopped
m.mu.RUnlock()
if stopped {
return nil, errManagerStopped
}
if parentTmpDir == "" {
return nil, errors.New("host utility manager not started")
}
workDir := filepath.Join(parentTmpDir, agentType)
if err := os.MkdirAll(workDir, 0o755); err != nil {
return nil, fmt.Errorf("mkdir %s: %w", workDir, err)
}
@@ -266,7 +352,14 @@ func (m *Manager) createInstance(ctx context.Context, agentType string) (*instan
healthCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
defer cancel()
if err := waitForClientHealthy(healthCtx, client); err != nil {
_ = m.controlClient.DeleteInstance(context.Background(), resp.ID)
deleteCtx, deleteCancel := hostUtilityDeleteContext(ctx)
m.deleteInstance(deleteCtx, &instance{
agentType: agentType,
instanceID: resp.ID,
workDir: workDir,
client: client,
})
deleteCancel()
return nil, fmt.Errorf("instance %s not healthy: %w", resp.ID, err)
}
@@ -303,6 +396,7 @@ func waitForClientHealthy(ctx context.Context, c *agentctlclient.Client) error {
// installed on the host. The Refresh path uses it to classify Status
// accurately instead of lumping every error into StatusFailed.
var errAgentNotInstalled = errors.New("agent not installed")
var errManagerStopped = errors.New("host utility manager stopped")
// getInstance returns the warm instance for the agent type, lazily recreating
// it if missing (e.g. after a previous failure or crash).
@@ -320,13 +414,18 @@ func (m *Manager) getInstance(ctx context.Context, agentType string) (*instance,
}
m.mu.RLock()
if m.stopped {
m.mu.RUnlock()
return nil, nil, errManagerStopped
}
inst := m.instances[agentType]
parentTmpDir := m.parentTmpDir
m.mu.RUnlock()
if inst != nil {
return inst, ia, nil
}
if m.parentTmpDir == "" {
if parentTmpDir == "" {
return nil, nil, errors.New("host utility manager not started")
}
@@ -351,6 +450,13 @@ func (m *Manager) getInstance(ctx context.Context, agentType string) (*instance,
return nil, cerr
}
m.mu.Lock()
if m.stopped {
m.mu.Unlock()
deleteCtx, cancel := hostUtilityDeleteContext(ctx)
m.deleteInstance(deleteCtx, created)
cancel()
return nil, errManagerStopped
}
m.instances[agentType] = created
m.mu.Unlock()
return created, nil
@@ -0,0 +1,144 @@
package hostutility
import (
"context"
"os"
"path/filepath"
"sync"
"testing"
"time"
"github.com/kandev/kandev/internal/agent/agents"
"github.com/kandev/kandev/internal/agent/registry"
"github.com/kandev/kandev/internal/agent/usage"
"github.com/kandev/kandev/internal/common/logger"
"github.com/kandev/kandev/pkg/agent"
"github.com/stretchr/testify/require"
)
func TestStopCancelsInFlightBootstrap(t *testing.T) {
log := newTestLogger(t)
reg := registry.NewRegistry(log)
started := make(chan struct{})
canceled := make(chan struct{})
require.NoError(t, reg.Register(&blockingInferenceAgent{
started: started,
canceled: canceled,
}))
mgr := NewManager(reg, "127.0.0.1", 1, nil, log)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
done := make(chan error, 1)
go func() {
done <- mgr.Start(ctx)
}()
select {
case <-started:
case <-time.After(time.Second):
t.Fatal("host utility bootstrap did not start installation check")
}
mgr.Stop(context.Background())
select {
case <-canceled:
case <-time.After(500 * time.Millisecond):
t.Fatal("host utility Stop did not cancel in-flight bootstrap")
}
select {
case err := <-done:
require.NoError(t, err)
case <-time.After(time.Second):
t.Fatal("host utility Start did not return after Stop")
}
}
func TestDeleteInstanceRemovesWorkDirWithoutControlClient(t *testing.T) {
log := newTestLogger(t)
mgr := NewManager(registry.NewRegistry(log), "127.0.0.1", 1, nil, log)
workDir := filepath.Join(t.TempDir(), "codex-acp")
require.NoError(t, os.MkdirAll(workDir, 0o755))
mgr.deleteInstance(context.Background(), &instance{
agentType: "codex-acp",
workDir: workDir,
})
_, err := os.Stat(workDir)
require.ErrorIs(t, err, os.ErrNotExist)
}
func TestHostUtilityDeleteContextIgnoresParentCancellation(t *testing.T) {
parent, cancelParent := context.WithCancel(context.Background())
cancelParent()
ctx, cancel := hostUtilityDeleteContext(parent)
defer cancel()
select {
case <-ctx.Done():
t.Fatal("delete context should not inherit parent cancellation")
default:
}
}
func newTestLogger(t *testing.T) *logger.Logger {
t.Helper()
log, err := logger.NewLogger(logger.LoggingConfig{
Level: "error",
Format: "json",
})
if err != nil {
t.Fatalf("failed to create logger: %v", err)
}
return log
}
type blockingInferenceAgent struct {
once sync.Once
started chan struct{}
canceled chan struct{}
}
func (a *blockingInferenceAgent) ID() string { return "blocking-acp" }
func (a *blockingInferenceAgent) Name() string { return "Blocking ACP" }
func (a *blockingInferenceAgent) DisplayName() string { return "Blocking ACP" }
func (a *blockingInferenceAgent) Description() string { return "blocking test agent" }
func (a *blockingInferenceAgent) Enabled() bool { return true }
func (a *blockingInferenceAgent) DisplayOrder() int { return 1 }
func (a *blockingInferenceAgent) Logo(agents.LogoVariant) []byte {
return nil
}
func (a *blockingInferenceAgent) IsInstalled(ctx context.Context) (*agents.DiscoveryResult, error) {
a.once.Do(func() {
close(a.started)
})
<-ctx.Done()
close(a.canceled)
return nil, ctx.Err()
}
func (a *blockingInferenceAgent) BuildCommand(agents.CommandOptions) agents.Command {
return agents.NewCommand("blocking-acp")
}
func (a *blockingInferenceAgent) PermissionSettings() map[string]agents.PermissionSetting {
return nil
}
func (a *blockingInferenceAgent) Runtime() *agents.RuntimeConfig {
return &agents.RuntimeConfig{Protocol: agent.ProtocolACP}
}
func (a *blockingInferenceAgent) BillingType() usage.BillingType {
return usage.BillingTypeSubscription
}
func (a *blockingInferenceAgent) RemoteAuth() *agents.RemoteAuth { return nil }
func (a *blockingInferenceAgent) InstallScript() string { return "" }
func (a *blockingInferenceAgent) InferenceConfig() *agents.InferenceConfig {
return &agents.InferenceConfig{
Supported: true,
Command: agents.NewCommand("blocking-acp"),
}
}
@@ -0,0 +1,11 @@
package hostutility
import (
"testing"
"go.uber.org/goleak"
)
func TestMain(m *testing.M) {
goleak.VerifyTestMain(m)
}
@@ -55,10 +55,10 @@ type CreateInstanceRequest struct {
AssumeMcpSse bool `json:"assume_mcp_sse,omitempty"` // Assume agent supports SSE MCP servers
AssumeMcpHttp bool `json:"assume_mcp_http,omitempty"` // Assume agent supports HTTP MCP servers
McpMode string `json:"mcp_mode,omitempty"` // MCP tool mode: "task" (default), "config", or "office"
// RequiresProcessKill forces agentctl to kill the agent's process group
// (not just close stdin) on shutdown. Required for agents whose runtime
// keeps child processes (e.g. MCP servers) alive when stdin closes —
// notably opencode acp.
// RequiresProcessKill tells agentctl to skip the graceful stdin-close wait
// and reap the agent process group immediately. Required for agents whose
// runtime keeps child processes (e.g. MCP servers) alive when stdin closes
// — notably opencode acp.
RequiresProcessKill bool `json:"requires_process_kill,omitempty"`
// BaseBranches maps RepositoryName → base branch ref for the task's
@@ -184,9 +184,7 @@ func (l *Launcher) Start(ctx context.Context) error {
l.mu.Unlock()
if err := l.waitForHealthy(ctx); err != nil {
if killErr := l.cmd.Process.Kill(); killErr != nil {
l.logger.Warn("failed to kill agentctl process after failed health check", zap.Error(killErr))
}
l.forceKill(l.cmd.Process.Pid)
l.closeParentPipe()
l.releaseChildLifecycle()
return fmt.Errorf("agentctl failed to become healthy: %w", err)
@@ -266,9 +264,7 @@ func (l *Launcher) performHandshake(ctx context.Context, nonce string) (string,
ctl := agentctlclient.NewControlClient(l.host, l.port, l.logger)
token, err := ctl.Handshake(ctx, nonce)
if err != nil {
if killErr := l.cmd.Process.Kill(); killErr != nil {
l.logger.Warn("failed to kill agentctl after handshake failure", zap.Error(killErr))
}
l.forceKill(l.cmd.Process.Pid)
l.closeParentPipe()
l.releaseChildLifecycle()
return "", fmt.Errorf("agentctl handshake failed: %w", err)
@@ -299,6 +295,7 @@ func (l *Launcher) Stop(ctx context.Context) error {
l.mu.Unlock()
l.logger.Info("stopping agentctl subprocess", zap.Int("pid", pid))
l.logger.Debug("agentctl subprocess stop requested", zap.Int("pid", pid))
// Close the liveness pipe first — this signals agentctl that the parent
// is shutting down, complementing the SIGTERM that follows.
@@ -6,8 +6,8 @@ import (
"fmt"
"os/exec"
"sync/atomic"
"unsafe"
"github.com/kandev/kandev/internal/agentctl/server/winproc"
"go.uber.org/zap"
"golang.org/x/sys/windows"
)
@@ -29,34 +29,18 @@ func (l *Launcher) installChildLifecycle(cmd *exec.Cmd) error {
return fmt.Errorf("installChildLifecycle: process not started")
}
job, err := createKillOnCloseJob()
job, err := winproc.InstallKillOnCloseJobForCommand(cmd)
if err != nil {
return err
}
procHandle, err := windows.OpenProcess(
windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE,
false,
uint32(cmd.Process.Pid),
)
if err != nil {
_ = windows.CloseHandle(job)
return fmt.Errorf("OpenProcess(pid=%d): %w", cmd.Process.Pid, err)
}
defer windows.CloseHandle(procHandle)
if err := windows.AssignProcessToJobObject(job, procHandle); err != nil {
_ = windows.CloseHandle(job)
return fmt.Errorf("AssignProcessToJobObject: %w", err)
}
// Defensive: if a previous lifecycle was somehow installed without a
// matching release (unexpected restart, future code change), close the
// stale handle instead of silently leaking it. Doing the swap *after* the
// new job is assigned avoids killing the new agentctl before we've stored
// its handle — the new job has KILL_ON_JOB_CLOSE so we must hold a
// reference at all times.
previous := atomic.SwapUintptr(&l.jobHandle, uintptr(job))
previous := atomic.SwapUintptr(&l.jobHandle, job.RawHandle())
if previous != 0 {
if err := windows.CloseHandle(windows.Handle(previous)); err != nil {
l.logger.Warn("failed to close stale job object handle", zap.Error(err))
@@ -85,28 +69,3 @@ func (l *Launcher) releaseChildLifecycle() {
l.logger.Warn("failed to close job object handle", zap.Error(err))
}
}
// createKillOnCloseJob creates an unnamed Job Object whose handle, when the
// last reference is closed by the OS, terminates every process assigned to it.
func createKillOnCloseJob() (windows.Handle, error) {
job, err := windows.CreateJobObject(nil, nil)
if err != nil {
return 0, fmt.Errorf("CreateJobObject: %w", err)
}
info := windows.JOBOBJECT_EXTENDED_LIMIT_INFORMATION{
BasicLimitInformation: windows.JOBOBJECT_BASIC_LIMIT_INFORMATION{
LimitFlags: windows.JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE,
},
}
if _, err := windows.SetInformationJobObject(
job,
windows.JobObjectExtendedLimitInformation,
uintptr(unsafe.Pointer(&info)),
uint32(unsafe.Sizeof(info)),
); err != nil {
_ = windows.CloseHandle(job)
return 0, fmt.Errorf("SetInformationJobObject: %w", err)
}
return job, nil
}
@@ -8,8 +8,8 @@ import (
"testing"
"time"
"github.com/kandev/kandev/internal/agentctl/server/winproc"
"github.com/kandev/kandev/internal/common/logger"
"golang.org/x/sys/windows"
)
func newTestLauncher(t *testing.T) *Launcher {
@@ -21,19 +21,6 @@ func newTestLauncher(t *testing.T) *Launcher {
return &Launcher{logger: log}
}
func TestCreateKillOnCloseJob(t *testing.T) {
job, err := createKillOnCloseJob()
if err != nil {
t.Fatalf("createKillOnCloseJob: %v", err)
}
if job == 0 {
t.Fatal("got null job handle")
}
if err := windows.CloseHandle(job); err != nil {
t.Errorf("CloseHandle: %v", err)
}
}
func TestReleaseChildLifecycle_NoHandleIsNoop(t *testing.T) {
l := newTestLauncher(t)
// Must not panic when no handle has been installed.
@@ -44,13 +31,20 @@ func TestReleaseChildLifecycle_NoHandleIsNoop(t *testing.T) {
}
func TestReleaseChildLifecycle_Idempotent(t *testing.T) {
job, err := createKillOnCloseJob()
cmd := exec.Command("cmd.exe", "/c", "exit", "0")
if err := cmd.Start(); err != nil {
t.Fatalf("start child: %v", err)
}
job, err := winproc.InstallKillOnCloseJobForCommand(cmd)
if err != nil {
t.Fatalf("createKillOnCloseJob: %v", err)
t.Fatalf("InstallKillOnCloseJobForCommand: %v", err)
}
if err := cmd.Wait(); err != nil {
t.Fatalf("wait child: %v", err)
}
l := newTestLauncher(t)
atomic.StoreUintptr(&l.jobHandle, uintptr(job))
atomic.StoreUintptr(&l.jobHandle, job.RawHandle())
l.releaseChildLifecycle()
// Second call must not double-close the handle.
@@ -8,18 +8,25 @@ import (
"go.uber.org/zap"
)
// gracefulStop sends SIGTERM to the process for graceful shutdown.
// gracefulStop sends SIGTERM to the process group for graceful shutdown.
// Falls back to SIGKILL if SIGTERM fails.
func (l *Launcher) gracefulStop(pid int) error {
if err := syscall.Kill(pid, syscall.SIGTERM); err != nil {
l.logger.Debug("agentctl subprocess SIGTERM requested", zap.Int("pgid", pid))
if err := syscall.Kill(-pid, syscall.SIGTERM); err != nil {
l.logger.Warn("failed to send SIGTERM, trying SIGKILL", zap.Error(err))
_ = syscall.Kill(pid, syscall.SIGKILL)
l.logger.Debug("agentctl subprocess SIGKILL requested",
zap.Int("pgid", pid),
zap.String("reason", "sigterm_failed"))
_ = syscall.Kill(-pid, syscall.SIGKILL)
return err
}
return nil
}
// forceKill sends SIGKILL to the agentctl process.
// forceKill sends SIGKILL to the agentctl process group.
func (l *Launcher) forceKill(pid int) {
_ = syscall.Kill(pid, syscall.SIGKILL)
l.logger.Debug("agentctl subprocess SIGKILL requested",
zap.Int("pgid", pid),
zap.String("reason", "force_kill"))
_ = syscall.Kill(-pid, syscall.SIGKILL)
}
@@ -0,0 +1,141 @@
//go:build !windows
package launcher
import (
"fmt"
"os"
"os/exec"
"path/filepath"
"runtime"
"strconv"
"strings"
"syscall"
"testing"
"time"
"github.com/kandev/kandev/internal/common/logger"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"go.uber.org/zap/zaptest/observer"
)
func TestLauncherLogsSignalAttempts(t *testing.T) {
core, observed := observer.New(zapcore.DebugLevel)
log, err := logger.NewFromZap(zap.New(core))
if err != nil {
t.Fatalf("NewFromZap: %v", err)
}
launcher := &Launcher{logger: log}
const missingPID = 1 << 30
if err := launcher.gracefulStop(missingPID); err == nil {
t.Fatal("expected missing process to fail graceful stop")
}
launcher.forceKill(missingPID)
for _, message := range []string{
"agentctl subprocess SIGTERM requested",
"agentctl subprocess SIGKILL requested",
} {
if !launcherLogsContain(observed, message) {
t.Fatalf("expected debug log %q, got %#v", message, observed.All())
}
}
}
func TestLauncherForceKillSignalsProcessGroup(t *testing.T) {
tmp := t.TempDir()
childPIDFile := filepath.Join(tmp, "child.pid")
cmd := exec.Command("sh", "-c", `sleep 30 & echo $! > "$CHILD_PID_FILE"; wait`)
cmd.Env = append(os.Environ(), "CHILD_PID_FILE="+childPIDFile)
cmd.SysProcAttr = buildSysProcAttr()
if err := cmd.Start(); err != nil {
t.Fatalf("start command: %v", err)
}
pgid := cmd.Process.Pid
var childPID int
t.Cleanup(func() {
_ = syscall.Kill(-pgid, syscall.SIGKILL)
_ = cmd.Wait()
})
waitUntilLauncherTest(t, time.Second, func() bool {
raw, err := os.ReadFile(childPIDFile)
if err != nil {
return false
}
pid, err := strconv.Atoi(strings.TrimSpace(string(raw)))
if err != nil {
return false
}
childPID = pid
return processExistsForLauncherTest(childPID)
}, "child process pid was not written or running")
launcher := &Launcher{logger: newLauncherTestLogger(t)}
launcher.forceKill(pgid)
waitUntilLauncherTest(t, 2*time.Second, func() bool {
return !processExistsForLauncherTest(childPID)
}, "forceKill did not kill process-group child %d", childPID)
}
func launcherLogsContain(logs *observer.ObservedLogs, message string) bool {
for _, entry := range logs.All() {
if entry.Message == message {
return true
}
}
return false
}
func newLauncherTestLogger(t *testing.T) *logger.Logger {
t.Helper()
log, err := logger.NewLogger(logger.LoggingConfig{
Level: "error",
Format: "json",
})
if err != nil {
t.Fatalf("new logger: %v", err)
}
return log
}
func processExistsForLauncherTest(pid int) bool {
if pid <= 0 {
return false
}
if runtime.GOOS == "linux" {
raw, err := os.ReadFile(fmt.Sprintf("/proc/%d/stat", pid))
if err != nil {
return false
}
s := string(raw)
idx := strings.LastIndex(s, ") ")
if idx == -1 || idx+2 >= len(s) {
return false
}
return s[idx+2] != 'Z'
}
proc, err := os.FindProcess(pid)
if err != nil {
return false
}
return proc.Signal(syscall.Signal(0)) == nil
}
func waitUntilLauncherTest(t *testing.T, timeout time.Duration, condition func() bool, format string, args ...any) {
t.Helper()
deadline := time.Now().Add(timeout)
for time.Now().Before(deadline) {
if condition() {
return
}
time.Sleep(20 * time.Millisecond)
}
if condition() {
return
}
t.Fatalf(format, args...)
}
@@ -16,8 +16,12 @@ func (l *Launcher) gracefulStop(pid int) error {
return err
}
// os.Interrupt on Windows sends CTRL_BREAK_EVENT
l.logger.Debug("agentctl subprocess interrupt requested", zap.Int("pid", pid))
if err := proc.Signal(os.Interrupt); err != nil {
l.logger.Warn("failed to send interrupt, trying kill", zap.Error(err))
l.logger.Debug("agentctl subprocess kill requested",
zap.Int("pid", pid),
zap.String("reason", "interrupt_failed"))
_ = proc.Kill()
return err
}
@@ -27,6 +31,9 @@ func (l *Launcher) gracefulStop(pid int) error {
// forceKill terminates the process on Windows.
func (l *Launcher) forceKill(pid int) {
if proc, err := os.FindProcess(pid); err == nil {
l.logger.Debug("agentctl subprocess kill requested",
zap.Int("pid", pid),
zap.String("reason", "force_kill"))
_ = proc.Kill()
}
}
@@ -1,9 +1,13 @@
//go:build !linux && !windows
//go:build unix && !linux
package launcher
import "syscall"
func buildSysProcAttr() *syscall.SysProcAttr {
return &syscall.SysProcAttr{}
return &syscall.SysProcAttr{
// Keep standalone agentctl out of the terminal foreground process
// group so Ctrl+C is sequenced by the backend shutdown path.
Setpgid: true,
}
}
@@ -6,6 +6,9 @@ import "syscall"
func buildSysProcAttr() *syscall.SysProcAttr {
return &syscall.SysProcAttr{
// Keep standalone agentctl out of the terminal foreground process
// group so Ctrl+C is sequenced by the backend shutdown path.
Setpgid: true,
Pdeathsig: syscall.SIGTERM,
}
}
@@ -0,0 +1,9 @@
//go:build !unix && !windows
package launcher
import "syscall"
func buildSysProcAttr() *syscall.SysProcAttr {
return &syscall.SysProcAttr{}
}
@@ -1,4 +1,4 @@
//go:build !windows
//go:build unix
package launcher
@@ -6,9 +6,9 @@ import (
"testing"
)
func TestBuildSysProcAttr_NoSetpgid(t *testing.T) {
func TestBuildSysProcAttr_IsolatesAgentctlFromTerminalInterrupt(t *testing.T) {
attr := buildSysProcAttr()
if attr.Setpgid {
t.Error("Setpgid must be false: standalone agentctl should share the backend's process group")
if !attr.Setpgid {
t.Error("Setpgid must be true: standalone agentctl should not receive terminal Ctrl+C directly")
}
}
@@ -298,9 +298,10 @@ type ExecutorInstance struct {
StandalonePort int // Standalone
// Common fields
WorkspacePath string
Metadata map[string]interface{}
StopReason string
WorkspacePath string
Metadata map[string]interface{}
StopReason string
AgentStopFailed bool
// AuthToken is the agentctl auth token retrieved via handshake.
// Populated by Docker executor for encrypted storage in SecretStore.
@@ -22,6 +22,8 @@ import (
)
const dockerWorkspacePath = "/workspace"
const dockerStopContainerTimeout = 30 * time.Second
const dockerFallbackCleanupTimeout = dockerStopContainerTimeout + 5*time.Second
// getMetadataString retrieves a string value from metadata map.
func getMetadataString(metadata map[string]interface{}, key string) string {
@@ -595,7 +597,8 @@ func (r *DockerExecutor) StopInstance(ctx context.Context, instance *ExecutorIns
// the kandev-managed per-container session dir so we don't leak GBs of
// agent state on disk. Plain stops preserve the dir so resume re-attaches
// to the same agent state, mirroring the Sprites preserve-on-stop rule.
if shouldRunExecutorCleanup(instance.StopReason) && r.kandevHomeDir != "" && instance.InstanceID != "" {
teardownContainer := shouldTeardownDockerContainer(instance.StopReason)
if teardownContainer && r.kandevHomeDir != "" && instance.InstanceID != "" {
CleanupAgentSessionDir(InstanceSessionRoot(r.kandevHomeDir, instance.InstanceID), r.logger)
}
@@ -603,15 +606,31 @@ func (r *DockerExecutor) StopInstance(ctx context.Context, instance *ExecutorIns
return nil // No container to stop
}
// Plain "stop the agent" runs (e.g. user clicks Stop, then later wants to
// resume) must not stop the Docker container after agentctl stopped cleanly.
// The container holds the cloned workspace and agentctl process for fast
// resume; destructive lifecycle events or failed agentctl stops should tear
// it down.
if !force && !instance.AgentStopFailed && !teardownContainer {
r.logger.Info("preserving docker container after agent stop",
zap.String("container_id", instance.ContainerID),
zap.String("instance_id", instance.InstanceID),
zap.String("stop_reason", instance.StopReason))
return nil
}
dockerClient, _, err := r.ensureClient()
if err != nil {
return fmt.Errorf("docker unavailable: %w", err)
}
cleanupCtx, cancel := dockerCleanupContext(ctx, instance.AgentStopFailed)
defer cancel()
if force {
err = dockerClient.KillContainer(ctx, instance.ContainerID, "SIGKILL")
err = dockerClient.KillContainer(cleanupCtx, instance.ContainerID, "SIGKILL")
} else {
err = dockerClient.StopContainer(ctx, instance.ContainerID, 30*time.Second)
err = dockerClient.StopContainer(cleanupCtx, instance.ContainerID, dockerStopContainerTimeout)
}
if err != nil {
@@ -621,6 +640,25 @@ func (r *DockerExecutor) StopInstance(ctx context.Context, instance *ExecutorIns
return nil
}
// shouldTeardownDockerContainer extends terminal cleanup with stale execution
// cleanup for Docker only. A stale Docker execution owns a local container and
// per-instance session dir that become untracked before retry/resume launches a
// replacement. Sprites intentionally keep stale sandboxes; see
// shouldRunExecutorCleanup for that shared runtime policy.
func shouldTeardownDockerContainer(reason string) bool {
if shouldRunExecutorCleanup(reason) {
return true
}
return strings.ToLower(strings.TrimSpace(reason)) == stopReasonStaleExecutionCleanup
}
func dockerCleanupContext(ctx context.Context, agentStopFailed bool) (context.Context, context.CancelFunc) {
if agentStopFailed {
return context.WithTimeout(context.WithoutCancel(ctx), dockerFallbackCleanupTimeout)
}
return ctx, func() {}
}
func (r *DockerExecutor) RecoverInstances(_ context.Context) ([]*ExecutorInstance, error) {
// No-op: Docker client is initialized lazily on first use.
// If no session has used Docker yet, there's nothing to recover.
@@ -87,6 +87,87 @@ func TestDockerExecutor_GetInteractiveRunner(t *testing.T) {
}
}
func TestDockerStopInstancePreservesContainerOnPlainStop(t *testing.T) {
log := newTestDockerLogger()
exec := NewDockerExecutor(config.DockerConfig{}, "", log)
exec.newClientFunc = func(_ config.DockerConfig, _ *logger.Logger) (*docker.Client, error) {
t.Fatal("plain stop should not initialize docker client")
return nil, nil
}
if err := exec.StopInstance(context.Background(), &ExecutorInstance{
InstanceID: "inst-1",
ContainerID: "container-1",
StopReason: "stopped via API",
}, false); err != nil {
t.Fatalf("StopInstance: %v", err)
}
}
func TestDockerStopInstanceStopsContainerWhenAgentStopFailed(t *testing.T) {
log := newTestDockerLogger()
exec := NewDockerExecutor(config.DockerConfig{}, "", log)
exec.newClientFunc = failingClientFactory("docker required after agent stop failure")
err := exec.StopInstance(context.Background(), &ExecutorInstance{
InstanceID: "inst-1",
ContainerID: "container-1",
StopReason: "stopped via API",
AgentStopFailed: true,
}, false)
if err == nil {
t.Fatal("StopInstance should attempt Docker cleanup after agentctl stop failure")
}
if !strings.Contains(err.Error(), "docker required after agent stop failure") {
t.Fatalf("StopInstance error = %v", err)
}
}
func TestDockerStopInstanceStopsContainerOnStaleCleanup(t *testing.T) {
log := newTestDockerLogger()
exec := NewDockerExecutor(config.DockerConfig{}, "", log)
exec.newClientFunc = failingClientFactory("docker required for stale cleanup")
err := exec.StopInstance(context.Background(), &ExecutorInstance{
InstanceID: "inst-1",
ContainerID: "container-1",
StopReason: stopReasonStaleExecutionCleanup,
}, false)
if err == nil {
t.Fatal("StopInstance should attempt Docker cleanup for stale executions")
}
if !strings.Contains(err.Error(), "docker required for stale cleanup") {
t.Fatalf("StopInstance error = %v", err)
}
}
func TestDockerCleanupContextIgnoresCanceledParentAfterAgentStopFailed(t *testing.T) {
parent, parentCancel := context.WithCancel(context.Background())
parentCancel()
cleanupCtx, cleanupCancel := dockerCleanupContext(parent, true)
defer cleanupCancel()
if err := cleanupCtx.Err(); err != nil {
t.Fatalf("cleanup context should remain usable after parent cancellation, got %v", err)
}
if _, ok := cleanupCtx.Deadline(); !ok {
t.Fatal("cleanup context should keep its own timeout")
}
}
func TestDockerCleanupContextKeepsParentCancellationForNormalStops(t *testing.T) {
parent, parentCancel := context.WithCancel(context.Background())
cleanupCtx, cleanupCancel := dockerCleanupContext(parent, false)
defer cleanupCancel()
parentCancel()
if err := cleanupCtx.Err(); err != context.Canceled {
t.Fatalf("cleanup context error = %v, want context.Canceled", err)
}
}
func TestDockerExecutor_EnsureClient_Success(t *testing.T) {
log := newTestDockerLogger()
exec := NewDockerExecutor(config.DockerConfig{}, "", log)
@@ -132,7 +132,10 @@ func (r *SpritesExecutor) runTerminalCleanupScript(ctx context.Context, sprite *
// Terminal stop reasons that trigger destructive executor cleanup
// (sandbox teardown, container removal, per-instance session-dir removal).
// Anything outside this set is treated as a "preserve" stop — see
// shouldRunExecutorCleanup.
// shouldRunExecutorCleanup. Stale execution cleanup is intentionally excluded
// from this shared set: Docker has a runtime-specific helper to remove local
// stale containers, while Sprites preserves cloud sandboxes for faster and less
// destructive recovery.
const (
StopReasonTaskArchived = "task archived"
StopReasonTaskDeleted = "task deleted"
@@ -92,8 +92,7 @@ type Manager struct {
// shuttingDown is flipped true when graceful shutdown begins (see
// StopAllAgents) so handlers running in detached goroutines can
// short-circuit work that would otherwise race the teardown and log
// confusing errors against children that already died from the same
// terminal-wide SIGINT.
// confusing errors against children already being stopped.
shuttingDown atomic.Bool
// pollAggregator routes hub session-mode events to agentctl. See
@@ -521,12 +521,13 @@ func (m *Manager) StopAgentWithReason(ctx context.Context, executionID string, r
zap.Stringer("runtime", execution.RuntimeName))
// Try to gracefully stop via agentctl first, then always close connections
agentStopFailed := false
if execution.agentctl != nil {
if !force {
if err := execution.agentctl.Stop(ctx); err != nil {
// During shutdown agentctl typically received the same
// terminal-wide SIGINT and is already gone, so a failed
// HTTP call here is expected, not noteworthy.
agentStopFailed = true
// During shutdown the instance may already be stopping through
// another lifecycle path, so a failed HTTP call is expected.
if m.IsShuttingDown() {
m.logger.Debug("failed to stop agent via agentctl",
zap.String("execution_id", executionID),
@@ -542,7 +543,7 @@ func (m *Manager) StopAgentWithReason(ctx context.Context, executionID string, r
}
// Stop the agent execution via the runtime that created it
m.stopAgentViaBackend(ctx, executionID, execution, reason, force)
m.stopAgentViaBackend(ctx, executionID, execution, reason, force, agentStopFailed)
// Update execution status and remove from tracking
_ = m.executionStore.WithLock(executionID, func(exec *AgentExecution) {
@@ -1254,7 +1255,7 @@ func (m *Manager) RespondToPermissionBySessionID(sessionID, pendingID, optionID
}
// stopAgentViaBackend stops the agent execution via the runtime that created it.
func (m *Manager) stopAgentViaBackend(ctx context.Context, executionID string, execution *AgentExecution, reason string, force bool) {
func (m *Manager) stopAgentViaBackend(ctx context.Context, executionID string, execution *AgentExecution, reason string, force bool, agentStopFailed bool) {
if execution.RuntimeName == "" || m.executorRegistry == nil {
return
}
@@ -1275,11 +1276,11 @@ func (m *Manager) stopAgentViaBackend(ctx context.Context, executionID string, e
StandalonePort: execution.standalonePort,
Metadata: execution.Metadata,
StopReason: reason,
AgentStopFailed: agentStopFailed,
}
if err := rt.StopInstance(ctx, runtimeInstance, force); err != nil {
// During shutdown the runtime instance (e.g. a standalone agentctl)
// often already exited via the shared SIGINT, so StopInstance returns
// a benign 404. Only surface this at WARN outside shutdown.
// During shutdown the runtime instance may already be stopping or
// absent. Only surface this at WARN outside shutdown.
if m.IsShuttingDown() {
m.logger.Debug("failed to stop runtime instance, continuing with cleanup",
zap.String("execution_id", executionID),
@@ -223,6 +223,8 @@ func (m *Manager) StopAllAgents(ctx context.Context) error {
return errors.Join(errs...)
}
const stopReasonStaleExecutionCleanup = "stale execution cleanup"
// cleanupExitedContainer handles cleanup for a single exited container.
func (m *Manager) cleanupExitedContainer(ctx context.Context, containerID string) {
execution, tracked := m.executionStore.GetByContainerID(containerID)
@@ -313,7 +315,7 @@ func (m *Manager) CleanupStaleExecutionBySessionID(ctx context.Context, sessionI
// goroutines. Without this, the old agentctl instance keeps running when a new
// execution is created for the same session, causing git polling on deleted worktrees.
// This is idempotent — returns success if the instance is already gone.
m.stopAgentViaBackend(ctx, execution.ID, execution, "stale execution cleanup", false)
m.stopAgentViaBackend(ctx, execution.ID, execution, stopReasonStaleExecutionCleanup, false, false)
// Close agentctl connection if it exists
if execution.agentctl != nil {
+18 -5
View File
@@ -71,20 +71,33 @@ Kandev renders subagents (the `Task` tool) as cards and *wants* to nest each sub
Claude is the one that works today: `claude-agent-acp` (since PR #341) sets `_meta.claudeCode.parentToolUseId` on a subagent's internal calls, and its value already equals the parent Task tool_call id — so it maps straight onto our `parent_tool_call_id`. Cursor exposes nothing to nest. OpenCode needs a kandev-side child-session merge. (Top-level `parentToolCallId` is NOT in the ACP schema — `_meta` is the spec-compliant carrier.)
## Process-group kill for HTTP-server agents (`RequiresProcessKill`)
## Process-group cleanup and `RequiresProcessKill`
Most ACP agents (Claude Code, Codex, Cursor, …) exit cleanly when stdin is closed. Some agents — notably `opencode acp` — keep their HTTP server and an MCP child tree alive after stdin EOF. For those agents, closing stdin on shutdown is not enough; the process group has to be SIGKILLed so the MCP children don't re-parent to init and leak (GH issue #1247).
All ACP agents are launched in their own process group, and shutdown must not
report success while any process in that group is still alive. Some agents —
notably `opencode acp` — keep their HTTP server and an MCP child tree alive after
stdin EOF. For those agents, closing stdin on shutdown is not useful; the process
group has to be killed immediately so the MCP children don't re-parent to init
and leak (GH issue #1247).
The signal flows agent → adapter → process manager via a single bool:
The "skip graceful wait" signal flows agent → adapter → process manager via a
single bool:
1. `agents.RuntimeConfig.RequiresProcessKill` (set `true` on `OpenCodeACP`).
2. The lifecycle executors copy it into `agentctl.CreateInstanceRequest.RequiresProcessKill`.
3. `instance.Manager` stores it on `config.InstanceConfig.RequiresProcessKill`.
4. `process.Manager.buildAdapterConfig` forwards it into `adapter.Config` → `shared.Config`.
5. `acp.Adapter.RequiresProcessKill()` returns `cfg.RequiresProcessKill`.
6. On shutdown, `process.Manager.killProcessGroupIfRequired` consults the adapter and, when true, sends SIGKILL to the whole pgid via `killProcessGroup` (`syscall.Kill(-pid, SIGKILL)` on Unix). If `Stop(ctx)` further times out before the process exits, `waitForProcessExit`'s `ctx.Done` branch re-runs the pgid SIGKILL (falling back to a leader-only kill if the group call errors) — belt-and-braces for any future ACP CLI that decides to ignore stdin close.
6. On shutdown, `process.Manager.killProcessGroupIfRequired` consults the adapter and, when true, sends SIGKILL to the whole pgid via `killProcessGroup` (`syscall.Kill(-pid, SIGKILL)` on Unix).
To add another agent that needs this: set `RequiresProcessKill: true` in its `Runtime()` config — that's all.
`RequiresProcessKill: false` does **not** mean children may be left running. It
only allows the normal graceful path first (stdin close, adapter close, process
wait). After the command leader exits, `waitForProcessExit` still checks the
agent process group and sends SIGTERM/SIGKILL if descendants remain. If `Stop(ctx)`
times out, it also re-runs the pgid SIGKILL fallback.
To add another agent that needs immediate kill instead of graceful stdin close:
set `RequiresProcessKill: true` in its `Runtime()` config.
## Idle-instance reaper (`KANDEV_ACP_IDLE_TIMEOUT`)
@@ -216,9 +216,9 @@ type InstanceConfig struct {
// Inherited from the parent Config at instance creation time.
AuthToken string
// RequiresProcessKill forces the agent's process group to be killed on
// shutdown instead of relying on stdin close. Required for agents whose
// runtime keeps child processes alive when stdin closes (opencode acp).
// RequiresProcessKill skips the graceful stdin-close wait and reaps the
// agent's process group immediately. Required for agents whose runtime
// keeps child processes alive when stdin closes (opencode acp).
RequiresProcessKill bool
// BaseBranches maps RepositoryName → base branch ref for per-repo diff
@@ -4,7 +4,6 @@
package instance
import (
"net/http"
"sync/atomic"
"time"
@@ -39,7 +38,7 @@ type Instance struct {
manager *process.Manager
// server is the HTTP server for this instance's API (unexported)
server *http.Server
server instanceHTTPServer
// lastActivityNanos is the unix-nano timestamp of the most recently
// observed HTTP request on this instance's port. Used by the idle reaper
@@ -24,6 +24,8 @@ import (
// ServerFactory creates an HTTP handler for an instance given its config and process manager.
type ServerFactory func(cfg *config.InstanceConfig, procMgr *process.Manager, log *logger.Logger) http.Handler
const instanceHTTPShutdownGrace = 250 * time.Millisecond
// Manager manages multiple agent instances.
// It handles creation, tracking, and removal of agent instances,
// each with their own HTTP server on a dedicated port.
@@ -345,12 +347,10 @@ func (m *Manager) StopInstance(ctx context.Context, id string) error {
zap.String("instance_id", id),
zap.Int("port", inst.Port))
// Shutdown HTTP server (potentially slow, done without lock)
var stopErr error
if inst.server != nil {
if err := inst.server.Shutdown(ctx); err != nil {
m.logger.Warn("error shutting down HTTP server",
zap.String("instance_id", id),
zap.Error(err))
if err := m.stopHTTPServer(ctx, id, inst.Port, inst.server); err != nil {
stopErr = err
}
}
@@ -363,6 +363,10 @@ func (m *Manager) StopInstance(ctx context.Context, id string) error {
m.portAlloc.Release(inst.Port)
m.mu.Unlock()
if stopErr != nil {
return stopErr
}
m.logger.Info("StopInstance completed",
zap.String("instance_id", id),
zap.Int("port", inst.Port))
@@ -370,6 +374,35 @@ func (m *Manager) StopInstance(ctx context.Context, id string) error {
return nil
}
type instanceHTTPServer interface {
Shutdown(context.Context) error
Close() error
}
func (m *Manager) stopHTTPServer(ctx context.Context, id string, port int, server instanceHTTPServer) error {
serverCtx, cancel := context.WithTimeout(ctx, instanceHTTPShutdownGrace)
err := server.Shutdown(serverCtx)
cancel()
if err == nil || errors.Is(err, http.ErrServerClosed) {
return nil
}
m.logger.Debug("StopInstance: HTTP server graceful shutdown expired, closing active connections",
zap.String("instance_id", id),
zap.Int("port", port),
zap.Duration("grace", instanceHTTPShutdownGrace),
zap.Error(err))
if closeErr := server.Close(); closeErr != nil && !errors.Is(closeErr, http.ErrServerClosed) {
m.logger.Warn("error closing HTTP server",
zap.String("instance_id", id),
zap.Error(closeErr))
return fmt.Errorf("close HTTP server for instance %s: %w", id, closeErr)
}
if !errors.Is(err, context.DeadlineExceeded) && !errors.Is(err, context.Canceled) {
return fmt.Errorf("shutdown HTTP server for instance %s: %w", id, err)
}
return nil
}
// Shutdown stops all instances gracefully.
func (m *Manager) Shutdown(ctx context.Context) error {
// Stop the idle reaper first so it doesn't fire StopInstance concurrently
@@ -0,0 +1,172 @@
package instance
import (
"context"
"errors"
"net"
"net/http"
"testing"
"time"
"github.com/kandev/kandev/internal/agentctl/server/config"
"github.com/kandev/kandev/pkg/agent"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestStopInstanceBoundsHTTPServerShutdown(t *testing.T) {
log := newTestLogger(t)
mgr := NewManager(&config.Config{
Ports: config.PortConfig{Base: 0, Max: 0},
Defaults: config.InstanceDefaults{Protocol: agent.ProtocolACP},
}, log)
t.Cleanup(func() {
_ = mgr.Shutdown(context.Background())
})
requestStarted := make(chan struct{})
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
close(requestStarted)
select {
case <-r.Context().Done():
return
case <-time.After(2 * time.Second):
w.WriteHeader(http.StatusOK)
}
})
listener, err := net.Listen("tcp", "127.0.0.1:0")
require.NoError(t, err)
server := &http.Server{Handler: handler}
serveErr := make(chan error, 1)
go func() {
serveErr <- server.Serve(listener)
}()
inst := &Instance{
ID: "slow-http-shutdown",
Port: listener.Addr().(*net.TCPAddr).Port,
Status: "running",
CreatedAt: time.Now(),
server: server,
}
mgr.mu.Lock()
mgr.instances[inst.ID] = inst
mgr.mu.Unlock()
clientDone := make(chan struct{})
go func() {
defer close(clientDone)
resp, err := http.Get("http://" + listener.Addr().String() + "/probe")
if err == nil {
_ = resp.Body.Close()
}
}()
select {
case <-requestStarted:
case <-time.After(time.Second):
t.Fatal("test HTTP request did not reach server")
}
start := time.Now()
require.NoError(t, mgr.StopInstance(context.Background(), inst.ID))
elapsed := time.Since(start)
assert.Less(t, elapsed, 700*time.Millisecond,
"StopInstance should not wait for long-running probe handlers during shutdown")
select {
case <-clientDone:
case <-time.After(time.Second):
t.Fatal("test HTTP client did not finish after instance shutdown")
}
select {
case err := <-serveErr:
require.ErrorIs(t, err, http.ErrServerClosed)
case <-time.After(time.Second):
t.Fatal("test HTTP server did not stop")
}
}
func TestStopHTTPServerReturnsCloseError(t *testing.T) {
log := newTestLogger(t)
mgr := NewManager(&config.Config{
Ports: config.PortConfig{Base: 0, Max: 0},
Defaults: config.InstanceDefaults{Protocol: agent.ProtocolACP},
}, log)
closeErr := errors.New("listener close failed")
server := &fakeHTTPServer{
shutdownErr: context.DeadlineExceeded,
closeErr: closeErr,
}
err := mgr.stopHTTPServer(context.Background(), "close-failure", 12345, server)
require.ErrorIs(t, err, closeErr)
require.True(t, server.closed)
}
func TestStopInstanceReleasesPortWhenHTTPServerCloseFails(t *testing.T) {
log := newTestLogger(t)
mgr := NewManager(&config.Config{
Ports: config.PortConfig{Base: 12345, Max: 12345},
Defaults: config.InstanceDefaults{Protocol: agent.ProtocolACP},
}, log)
port, err := mgr.portAlloc.Allocate("close-failure")
require.NoError(t, err)
closeErr := errors.New("listener close failed")
inst := &Instance{
ID: "close-failure",
Port: port,
Status: "running",
CreatedAt: time.Now(),
server: &fakeHTTPServer{
shutdownErr: context.DeadlineExceeded,
closeErr: closeErr,
},
}
mgr.mu.Lock()
mgr.instances[inst.ID] = inst
mgr.mu.Unlock()
err = mgr.StopInstance(context.Background(), inst.ID)
require.ErrorIs(t, err, closeErr)
reusedPort, err := mgr.portAlloc.Allocate("next-instance")
require.NoError(t, err)
require.Equal(t, port, reusedPort)
}
func TestStopHTTPServerTreatsCanceledShutdownAsStoppedAfterClose(t *testing.T) {
log := newTestLogger(t)
mgr := NewManager(&config.Config{
Ports: config.PortConfig{Base: 0, Max: 0},
Defaults: config.InstanceDefaults{Protocol: agent.ProtocolACP},
}, log)
server := &fakeHTTPServer{shutdownErr: context.Canceled}
err := mgr.stopHTTPServer(context.Background(), "canceled-shutdown", 12345, server)
require.NoError(t, err)
require.True(t, server.closed)
}
type fakeHTTPServer struct {
shutdownErr error
closeErr error
closed bool
}
func (s *fakeHTTPServer) Shutdown(context.Context) error {
return s.shutdownErr
}
func (s *fakeHTTPServer) Close() error {
s.closed = true
return s.closeErr
}
@@ -404,6 +404,9 @@ func (r *InteractiveRunner) Stop(ctx context.Context, processID string) error {
// Terminate the process directly (PTY handles its own session management)
if proc.cmd != nil && proc.cmd.Process != nil {
r.logger.Debug("interactive process terminate requested",
zap.String("process_id", processID),
zap.Int("pid", pid))
_ = terminateProcess(proc.cmd.Process)
// Wait for the wait() goroutine to finish (it calls cmd.Wait).
@@ -414,11 +417,19 @@ func (r *InteractiveRunner) Stop(ctx context.Context, processID string) error {
zap.String("process_id", processID),
zap.Int("os_pid", pid),
zap.Error(ctx.Err()))
r.logger.Debug("interactive process SIGKILL requested",
zap.String("process_id", processID),
zap.Int("pid", pid),
zap.String("reason", "context_canceled"))
_ = proc.cmd.Process.Kill()
case <-time.After(2 * time.Second):
r.logger.Warn("interactive process stop timed out; killing process",
zap.String("process_id", processID),
zap.Int("os_pid", pid))
r.logger.Debug("interactive process SIGKILL requested",
zap.String("process_id", processID),
zap.Int("pid", pid),
zap.String("reason", "grace_expired"))
_ = proc.cmd.Process.Kill()
case <-proc.waitDone:
// Process exited cleanly
@@ -70,19 +70,31 @@ const defaultStderrBufferSize = 50
const agentTempDirRoot = "kandev-agent"
const processKillRequiredExitGrace = 250 * time.Millisecond
// processDefaultExitGrace is the no-deadline upper bound for graceful adapters
// after adapter/stdin close. It is intentionally longer than the kill-required
// fast path so ACP-style agents can flush state, while still bounding callers
// that pass context.Background().
const processDefaultExitGrace = 5 * time.Second
const processGroupTerminateGrace = 2 * time.Second
const processGroupPollInterval = 50 * time.Millisecond
// Manager manages the agent subprocess
type Manager struct {
cfg *config.InstanceConfig
logger *logger.Logger
// Process state
cmd *exec.Cmd
stdin io.WriteCloser
stdout io.ReadCloser
stderr io.ReadCloser
status atomic.Value // Status
exitCode atomic.Int32
exitErr atomic.Value // error
cmd *exec.Cmd
processLifecycle processLifecycleHandle
stdin io.WriteCloser
stdout io.ReadCloser
stderr io.ReadCloser
status atomic.Value // Status
exitCode atomic.Int32
exitErr atomic.Value // error
// Stderr buffering for error context
stderrBuffer []string
@@ -733,12 +745,23 @@ func (m *Manager) Start(ctx context.Context) error {
m.status.Store(StatusError)
return formatAgentStartError(err, m.cfg.AgentEnv)
}
processLifecycle, err := installProcessLifecycle(m.cmd)
if err != nil {
m.logger.Warn("failed to install agent process lifecycle; falling back to process-tree cleanup",
zap.Error(err))
} else {
m.processLifecycle = processLifecycle
}
m.stopCh = make(chan struct{})
m.doneCh = make(chan struct{})
// Connect adapter to the process stdin/stdout pipes
if err := m.adapter.Connect(m.stdin, m.stdout); err != nil {
releaseProcessLifecycle(m.processLifecycle)
m.processLifecycle = processLifecycleHandle{}
_ = m.cmd.Process.Kill()
_ = m.cmd.Wait()
m.status.Store(StatusError)
return fmt.Errorf("failed to connect adapter: %w", err)
}
@@ -882,7 +905,7 @@ func (m *Manager) buildFinalCommand() error {
// Create a new process group so we can kill all child processes together.
// This is important for adapters like OpenCode that spawn child processes
// (npx -> sh -> node -> opencode binary).
setProcGroup(m.cmd)
setAgentProcGroup(m.cmd)
envBytes, largestEnv := summarizeEnvBytes(m.cfg.AgentEnv, 3)
m.logger.Info("agent command prepared",
@@ -1214,16 +1237,28 @@ func (m *Manager) Stop(ctx context.Context) error {
m.mu.Lock()
defer m.mu.Unlock()
// Stop intentionally serializes the full teardown under m.mu. The shutdown
// path closes adapters, stdin, shell sessions, and process groups as one
// lifecycle transition; concurrent readers may briefly block while process
// group reaping finishes.
// Stop trackers before the status guard: passthrough never calls Start() so the early return below would otherwise leak them.
m.stopWorkspaceTrackers()
status := m.Status()
if status == StatusStopped || status == StatusStopping {
m.logger.Info("Stop called but already stopped/stopping", zap.String("status", string(status)))
m.logger.Info("Stop called but already stopped/stopping",
zap.String("status", string(status)),
zap.Int("pid", m.agentPID()))
return nil
}
m.logger.Info("stopping agent process - START")
m.logger.Info("stopping agent process - START",
zap.Int("pid", m.agentPID()),
zap.String("protocol", m.agentProtocol()))
m.logger.Debug("agent process stop requested",
zap.Int("pid", m.agentPID()),
zap.String("protocol", m.agentProtocol()))
m.status.Store(StatusStopping)
m.stopShellAndProcesses(ctx)
@@ -1236,6 +1271,20 @@ func (m *Manager) Stop(ctx context.Context) error {
return nil
}
func (m *Manager) agentPID() int {
if m.cmd == nil || m.cmd.Process == nil {
return 0
}
return m.cmd.Process.Pid
}
func (m *Manager) agentProtocol() string {
if m.cfg == nil {
return ""
}
return string(m.cfg.Protocol)
}
// stopShellAndProcesses stops the shell session, VS Code, and workspace processes.
func (m *Manager) stopShellAndProcesses(ctx context.Context) {
// Stop VS Code server if running
@@ -1294,8 +1343,10 @@ func (m *Manager) closeAdapterAndStdin() {
m.logger.Debug("stdin closed")
}
// killProcessGroupIfRequired kills the entire process group for adapters (such as
// OpenCode) that run as HTTP servers and do not exit when stdin is closed.
// killProcessGroupIfRequired immediately kills the entire process group for
// adapters (such as OpenCode) that are known not to exit when stdin is closed.
// Other adapters still get process-group cleanup in waitForProcessExit after
// their graceful stdin-close path has had a chance to finish.
func (m *Manager) killProcessGroupIfRequired() {
if m.adapter == nil || !m.adapter.RequiresProcessKill() {
return
@@ -1306,9 +1357,14 @@ func (m *Manager) killProcessGroupIfRequired() {
// We kill the process group to ensure all child processes are killed too.
// This is important because OpenCode spawns: npx -> sh -> node -> opencode binary
pid := m.cmd.Process.Pid
m.logger.Debug("killing process group", zap.Int("pgid", pid))
m.logger.Debug("agent process group SIGKILL requested",
zap.Int("pgid", pid),
zap.String("reason", "adapter_requires_process_kill"))
if err := killProcessGroup(pid); err != nil {
m.logger.Debug("failed to kill process group, trying single process", zap.Error(err))
m.logger.Debug("agent process SIGKILL requested",
zap.Int("pid", pid),
zap.String("reason", "process_group_kill_failed"))
if err := m.cmd.Process.Kill(); err != nil {
m.logger.Warn("failed to kill process", zap.Error(err))
}
@@ -1319,9 +1375,11 @@ func (m *Manager) killProcessGroupIfRequired() {
//
// On timeout the entire process group is killed (not just the leader) so that
// child processes — most importantly MCP servers spawned by the agent — don't
// re-parent to init and leak. setProcGroup at command-build time puts the
// agent in its own pgid; here we deliver SIGKILL to that pgid. Falls back to
// a single-process kill only if the process-group call fails.
// re-parent to init and leak. setAgentProcGroup at command-build time puts the
// agent in its own pgid; here we deliver SIGKILL to that pgid. If the command
// leader exits before its descendants do, we still terminate the remaining
// process group before reporting shutdown complete. Falls back to a
// single-process kill only if the process-group call fails.
func (m *Manager) waitForProcessExit(ctx context.Context) {
m.logger.Debug("waiting for process to exit")
done := make(chan struct{})
@@ -1330,21 +1388,176 @@ func (m *Manager) waitForProcessExit(ctx context.Context) {
close(done)
}()
pid := 0
if m.cmd != nil && m.cmd.Process != nil {
pid = m.cmd.Process.Pid
}
exitGrace := m.processExitGrace(ctx)
if waitForManagerDone(ctx, done, exitGrace) {
m.logger.Info("agent process stopped gracefully")
m.reapRemainingProcessGroup(ctx, pid)
return
}
if pid == 0 {
return
}
if ctx.Err() != nil {
m.logger.Warn("force killing agent process group", zap.Int("pgid", pid))
m.forceKillProcessGroupAndWait(done, pid)
return
}
m.logger.Warn("agent process did not exit after graceful wait; terminating process group",
zap.Int("pgid", pid),
zap.Duration("grace", exitGrace))
m.logger.Debug("agent process group SIGTERM requested",
zap.Int("pgid", pid),
zap.String("reason", "graceful_wait_expired"),
zap.Duration("grace", exitGrace))
if err := terminateProcessGroup(pid); err != nil {
if !isProcessGroupMissing(err) {
m.logger.Warn("failed to terminate agent process group, force killing",
zap.Int("pgid", pid),
zap.Error(err))
m.forceKillProcessGroupAndWait(done, pid)
}
return
}
if waitForManagerDone(ctx, done, processGroupTerminateGrace) {
m.logger.Info("agent process stopped after process group termination",
zap.Int("pgid", pid))
m.reapRemainingProcessGroup(ctx, pid)
return
}
m.logger.Warn("agent process did not stop after termination; force killing process group",
zap.Int("pgid", pid))
m.forceKillProcessGroupAndWait(done, pid)
}
// processExitGrace returns the initial graceful wait before process-group
// termination. Adapters that declare RequiresProcessKill are known not to exit
// from stdin close, so they keep the short legacy wait. Normal adapters get the
// caller's remaining deadline during backend shutdown, or processDefaultExitGrace
// when the caller did not provide one.
func (m *Manager) processExitGrace(ctx context.Context) time.Duration {
if m.adapter != nil && m.adapter.RequiresProcessKill() {
return processKillRequiredExitGrace
}
if deadline, ok := ctx.Deadline(); ok {
if remaining := time.Until(deadline); remaining > 0 {
return remaining
}
return 0
}
return processDefaultExitGrace
}
func waitForManagerDone(ctx context.Context, done <-chan struct{}, timeout time.Duration) bool {
timer := time.NewTimer(timeout)
defer timer.Stop()
select {
case <-done:
m.logger.Info("agent process stopped gracefully")
return true
case <-ctx.Done():
if m.cmd == nil || m.cmd.Process == nil {
return false
case <-timer.C:
return false
}
}
func (m *Manager) reapRemainingProcessGroup(ctx context.Context, pid int) {
if pid == 0 || !processGroupAlive(pid) {
return
}
m.logger.Warn("agent process group still alive after leader exit; terminating",
zap.Int("pgid", pid))
m.logger.Debug("agent process group SIGTERM requested",
zap.Int("pgid", pid),
zap.String("reason", "leader_exited_with_live_descendants"))
if err := terminateProcessGroup(pid); err != nil {
if isProcessGroupMissing(err) {
return
}
pid := m.cmd.Process.Pid
m.logger.Warn("force killing agent process group", zap.Int("pgid", pid))
if err := killProcessGroup(pid); err != nil {
m.logger.Warn("failed to terminate agent process group, force killing",
zap.Int("pgid", pid),
zap.Error(err))
m.forceKillProcessGroupAndWait(nil, pid)
return
}
waitCtx, cancel := context.WithTimeout(ctx, processGroupTerminateGrace)
defer cancel()
if waitForProcessGroupExit(waitCtx, pid) {
m.logger.Info("agent process group stopped after termination",
zap.Int("pgid", pid))
return
}
m.logger.Warn("agent process group did not stop after termination; force killing",
zap.Int("pgid", pid))
m.forceKillProcessGroupAndWait(nil, pid)
}
func (m *Manager) forceKillProcessGroup(pid int) {
m.logger.Debug("agent process group SIGKILL requested",
zap.Int("pgid", pid),
zap.String("reason", "force_kill"))
if err := killProcessGroup(pid); err != nil {
if isProcessGroupMissing(err) {
return
}
if m.cmd != nil && m.cmd.Process != nil {
m.logger.Warn("failed to kill agent process group, falling back to single-process kill",
zap.Error(err))
m.logger.Debug("agent process SIGKILL requested",
zap.Int("pid", m.cmd.Process.Pid),
zap.String("reason", "process_group_kill_failed"))
if err := m.cmd.Process.Kill(); err != nil {
m.logger.Warn("failed to kill agent process", zap.Error(err))
}
return
}
m.logger.Warn("failed to kill agent process group", zap.Error(err))
}
}
func (m *Manager) forceKillProcessGroupAndWait(done <-chan struct{}, pid int) {
m.forceKillProcessGroup(pid)
waitCtx, cancel := context.WithTimeout(context.Background(), processGroupTerminateGrace)
defer cancel()
if done != nil && waitForManagerDone(waitCtx, done, processGroupTerminateGrace) {
m.logger.Info("agent process stopped after force kill",
zap.Int("pgid", pid))
m.reapRemainingProcessGroup(context.Background(), pid)
return
}
if waitForProcessGroupExit(waitCtx, pid) {
m.logger.Info("agent process group stopped after force kill",
zap.Int("pgid", pid))
return
}
m.logger.Warn("agent process group still alive after force kill wait",
zap.Int("pgid", pid),
zap.Duration("grace", processGroupTerminateGrace))
}
func waitForProcessGroupExit(ctx context.Context, pid int) bool {
if !processGroupAlive(pid) {
return true
}
ticker := time.NewTicker(processGroupPollInterval)
defer ticker.Stop()
for {
select {
case <-ctx.Done():
return !processGroupAlive(pid)
case <-ticker.C:
if !processGroupAlive(pid) {
return true
}
}
}
}
@@ -1412,7 +1625,10 @@ func (m *Manager) waitForExit() {
defer m.wg.Done()
defer close(m.doneCh)
pid := m.agentPID()
err := m.cmd.Wait()
releaseProcessLifecycle(m.processLifecycle)
m.processLifecycle = processLifecycleHandle{}
if err != nil {
m.exitErr.Store(errorWrapper{err: err})
@@ -1450,6 +1666,9 @@ func (m *Manager) waitForExit() {
m.logger.Info("agent process exited successfully")
}
if m.Status() != StatusStopping {
m.reapRemainingProcessGroup(context.Background(), pid)
}
m.status.Store(StatusStopped)
}
@@ -14,6 +14,7 @@ import (
"testing"
"time"
"github.com/kandev/kandev/internal/agentctl/server/adapter"
"github.com/stretchr/testify/require"
)
@@ -24,7 +25,7 @@ import (
// After the fix, killProcessGroup is delivered to the leader's pgid so the
// whole tree dies.
func TestWaitForProcessExit_KillsProcessGroupOnTimeout(t *testing.T) {
log := newTestLogger(t)
log, observed := newObservedTestLogger(t)
pidFile := filepath.Join(t.TempDir(), "child.pid")
m := &Manager{
@@ -68,6 +69,8 @@ func TestWaitForProcessExit_KillsProcessGroupOnTimeout(t *testing.T) {
cancel()
m.waitForProcessExit(ctx)
require.True(t, observedLogsContain(observed, "agent process group SIGKILL requested"),
"shutdown should log the process-group SIGKILL attempt")
// Both parent and child must be reaped within a short window. We waited
// the parent ourselves; for the child we treat both "process gone" and
@@ -82,6 +85,238 @@ func TestWaitForProcessExit_KillsProcessGroupOnTimeout(t *testing.T) {
"child process %d should be killed by process-group reap", childPID)
}
func TestWaitForProcessExit_ContextCanceledWaitsAfterForceKill(t *testing.T) {
log := newTestLogger(t)
pidFile := filepath.Join(t.TempDir(), "child.pid")
m := &Manager{
logger: log,
}
m.cmd = fixtureCmd("sleep-with-child " + pidFile + " 30")
setProcGroup(m.cmd)
require.NoError(t, m.cmd.Start())
parentPID := m.cmd.Process.Pid
waitDone := make(chan struct{})
m.wg.Add(1)
go func() {
defer m.wg.Done()
_ = m.cmd.Wait()
close(waitDone)
}()
t.Cleanup(func() {
_ = killProcessGroup(parentPID)
select {
case <-waitDone:
case <-time.After(5 * time.Second):
t.Fatalf("timed out waiting for fixture parent to exit")
}
})
childPID := waitForChildPID(t, pidFile, 5*time.Second)
ctx, cancel := context.WithCancel(context.Background())
cancel()
m.waitForProcessExit(ctx)
select {
case <-waitDone:
default:
t.Fatal("waitForProcessExit returned before command wait completed after force kill")
}
require.Eventually(t, func() bool {
return !processAlive(childPID)
}, 5*time.Second, 50*time.Millisecond,
"child process %d should be killed by process-group force kill", childPID)
}
func TestWaitForProcessExit_ReapsProcessGroupAfterLeaderExit(t *testing.T) {
log := newTestLogger(t)
pidFile := filepath.Join(t.TempDir(), "child.pid")
m := &Manager{
logger: log,
}
m.cmd = fixtureCmd("exit-with-child " + pidFile + " 30")
setProcGroup(m.cmd)
require.NoError(t, m.cmd.Start())
parentPID := m.cmd.Process.Pid
waitDone := make(chan struct{})
m.wg.Add(1)
go func() {
defer m.wg.Done()
_ = m.cmd.Wait()
close(waitDone)
}()
t.Cleanup(func() {
_ = killProcessGroup(parentPID)
select {
case <-waitDone:
case <-time.After(5 * time.Second):
t.Fatalf("timed out waiting for fixture parent to exit")
}
})
childPID := waitForChildPID(t, pidFile, 5*time.Second)
childPGID, err := syscall.Getpgid(childPID)
require.NoError(t, err)
require.Equal(t, parentPID, childPGID,
"child must remain in the leader's pgid after the leader exits")
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
m.waitForProcessExit(ctx)
require.Eventually(t, func() bool {
return !processAlive(childPID)
}, 5*time.Second, 50*time.Millisecond,
"child process %d should be reaped even after the group leader exits", childPID)
}
func TestWaitForExit_ReapsProcessGroupAfterNaturalLeaderExit(t *testing.T) {
log := newTestLogger(t)
pidFile := filepath.Join(t.TempDir(), "child.pid")
m := &Manager{
logger: log,
updatesCh: make(chan adapter.AgentEvent, 1),
doneCh: make(chan struct{}),
}
m.status.Store(StatusRunning)
m.cmd = fixtureCmd("exit-with-child " + pidFile + " 30")
setProcGroup(m.cmd)
require.NoError(t, m.cmd.Start())
parentPID := m.cmd.Process.Pid
t.Cleanup(func() {
_ = killProcessGroup(parentPID)
})
childPID := waitForChildPID(t, pidFile, 5*time.Second)
m.wg.Add(1)
go m.waitForExit()
select {
case <-m.doneCh:
case <-time.After(15 * time.Second):
t.Fatal("timed out waiting for process manager waitForExit")
}
require.Eventually(t, func() bool {
return !processAlive(childPID)
}, 5*time.Second, 50*time.Millisecond,
"child process %d should be reaped when the leader exits naturally", childPID)
}
func TestWaitForProcessExit_TerminatesBeforeParentContextDeadline(t *testing.T) {
log, observed := newObservedTestLogger(t)
m := &Manager{
logger: log,
adapter: &stubAdapter{requiresProcessKill: true},
}
m.cmd = fixtureCmd("sleep 30")
setProcGroup(m.cmd)
require.NoError(t, m.cmd.Start())
parentPID := m.cmd.Process.Pid
waitDone := make(chan struct{})
m.wg.Add(1)
go func() {
defer m.wg.Done()
_ = m.cmd.Wait()
close(waitDone)
}()
t.Cleanup(func() {
_ = killProcessGroup(parentPID)
select {
case <-waitDone:
case <-time.After(5 * time.Second):
t.Fatalf("timed out waiting for fixture parent to exit")
}
})
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
defer cancel()
startedAt := time.Now()
m.waitForProcessExit(ctx)
elapsed := time.Since(startedAt)
require.Less(t, elapsed, 700*time.Millisecond,
"shutdown should terminate a non-exiting agent without spending a full second in local grace")
require.True(t, observedLogsContain(observed, "agent process group SIGTERM requested"),
"shutdown should log the process-group SIGTERM attempt")
require.Eventually(t, func() bool {
return !processAlive(parentPID)
}, 5*time.Second, 50*time.Millisecond,
"agent process %d should be killed after local graceful wait", parentPID)
}
func TestProcessExitGraceUsesCallerDeadlineForGracefulAdapter(t *testing.T) {
m := &Manager{
adapter: &stubAdapter{},
}
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
defer cancel()
grace := m.processExitGrace(ctx)
require.Greater(t, grace, 1500*time.Millisecond)
require.LessOrEqual(t, grace, 2*time.Second)
}
func TestProcessExitGraceUsesDefaultForGracefulAdapterWithoutDeadline(t *testing.T) {
m := &Manager{
adapter: &stubAdapter{},
}
require.Equal(t, processDefaultExitGrace, m.processExitGrace(context.Background()))
}
func TestProcessExitGraceUsesShortGraceForKillRequiredAdapter(t *testing.T) {
m := &Manager{
adapter: &stubAdapter{requiresProcessKill: true},
}
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
defer cancel()
require.Equal(t, processKillRequiredExitGrace, m.processExitGrace(ctx))
}
func TestStop_DoesNotReapStaleProcessGroupWhenStatusAlreadyStopped(t *testing.T) {
log, observed := newObservedTestLogger(t)
m := &Manager{
logger: log,
}
m.status.Store(StatusStopped)
m.cmd = fixtureCmd("sleep 30")
setProcGroup(m.cmd)
require.NoError(t, m.cmd.Start())
parentPID := m.cmd.Process.Pid
waitDone := make(chan struct{})
go func() {
_ = m.cmd.Wait()
close(waitDone)
}()
t.Cleanup(func() {
_ = killProcessGroup(parentPID)
select {
case <-waitDone:
case <-time.After(5 * time.Second):
t.Fatalf("timed out waiting for fixture parent to exit")
}
})
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
require.NoError(t, m.Stop(ctx))
require.True(t, processAlive(parentPID), "stopped manager should not signal a cached process PID")
require.False(t, observedLogsContain(observed, "agent process group SIGTERM requested"),
"stopped manager should not attempt process-group termination")
}
// waitForChildPID polls pidFile until it contains a valid PID or timeout
// expires. Returns the parsed PID. Fails the test on timeout.
func waitForChildPID(t *testing.T, pidFile string, timeout time.Duration) int {
@@ -20,8 +20,9 @@ import (
// stubAdapter is a minimal AgentAdapter for testing the startup sequence.
type stubAdapter struct {
connectCalled bool
updatesCh chan adapter.AgentEvent
connectCalled bool
requiresProcessKill bool
updatesCh chan adapter.AgentEvent
}
func newStubAdapter() *stubAdapter {
@@ -50,7 +51,7 @@ func (s *stubAdapter) Close() error {
close(s.updatesCh)
return nil
}
func (s *stubAdapter) RequiresProcessKill() bool { return false }
func (s *stubAdapter) RequiresProcessKill() bool { return s.requiresProcessKill }
func TestStartProcessPipes_CreatesAllPipes(t *testing.T) {
m := &Manager{
@@ -134,8 +135,21 @@ func TestManager_Start_PipesCreatedBeforeProcessStart(t *testing.T) {
// Now start the process — this is the fix: Start() happens after pipes.
err = m.cmd.Start()
require.NoError(t, err)
waited := false
t.Cleanup(func() {
if waited || m.cmd == nil || m.cmd.Process == nil {
return
}
_ = killProcessGroup(m.cmd.Process.Pid)
_ = m.cmd.Process.Kill()
_ = m.cmd.Wait()
})
assert.NotNil(t, m.cmd.Process, "process should be running")
processLifecycle, err := installProcessLifecycle(m.cmd)
require.NoError(t, err)
defer releaseProcessLifecycle(processLifecycle)
// Adapter connects after process starts.
err = stub.Connect(m.stdin, m.stdout)
require.NoError(t, err)
@@ -144,6 +158,7 @@ func TestManager_Start_PipesCreatedBeforeProcessStart(t *testing.T) {
// Clean up: close stdin so cat exits, then wait.
_ = m.stdin.Close()
_ = m.cmd.Wait()
waited = true
}
func TestFormatAgentStartError_E2BIGIncludesEnvDiagnostics(t *testing.T) {
@@ -3,6 +3,7 @@
package process
import (
"errors"
"os/exec"
"syscall"
)
@@ -18,6 +19,18 @@ func setProcGroup(cmd *exec.Cmd) {
}
}
func setAgentProcGroup(cmd *exec.Cmd) {
setProcGroup(cmd)
}
type processLifecycleHandle struct{}
func installProcessLifecycle(_ *exec.Cmd) (processLifecycleHandle, error) {
return processLifecycleHandle{}, nil
}
func releaseProcessLifecycle(_ processLifecycleHandle) {}
// killProcessGroup kills the entire process group for the given PID.
// Returns nil if successful, or an error if the kill failed.
func killProcessGroup(pid int) error {
@@ -29,3 +42,15 @@ func killProcessGroup(pid int) error {
func terminateProcessGroup(pid int) error {
return syscall.Kill(-pid, syscall.SIGTERM)
}
func processGroupAlive(pid int) bool {
if pid <= 0 {
return false
}
err := syscall.Kill(-pid, 0)
return err == nil || errors.Is(err, syscall.EPERM)
}
func isProcessGroupMissing(err error) bool {
return errors.Is(err, syscall.ESRCH)
}
@@ -3,6 +3,7 @@
package process
import (
"errors"
"os/exec"
"syscall"
)
@@ -15,6 +16,18 @@ func setProcGroup(cmd *exec.Cmd) {
cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
}
func setAgentProcGroup(cmd *exec.Cmd) {
setProcGroup(cmd)
}
type processLifecycleHandle struct{}
func installProcessLifecycle(_ *exec.Cmd) (processLifecycleHandle, error) {
return processLifecycleHandle{}, nil
}
func releaseProcessLifecycle(_ processLifecycleHandle) {}
// killProcessGroup kills the entire process group for the given PID.
// Returns nil if successful, or an error if the kill failed.
func killProcessGroup(pid int) error {
@@ -26,3 +39,15 @@ func killProcessGroup(pid int) error {
func terminateProcessGroup(pid int) error {
return syscall.Kill(-pid, syscall.SIGTERM)
}
func processGroupAlive(pid int) bool {
if pid <= 0 {
return false
}
err := syscall.Kill(-pid, 0)
return err == nil || errors.Is(err, syscall.EPERM)
}
func isProcessGroupMissing(err error) bool {
return errors.Is(err, syscall.ESRCH)
}
@@ -3,34 +3,69 @@
package process
import (
"errors"
"fmt"
"os/exec"
"syscall"
"github.com/kandev/kandev/internal/agentctl/server/winproc"
"golang.org/x/sys/windows"
)
// setProcGroup configures the command to run in its own process group.
// On Windows, we use CREATE_NEW_PROCESS_GROUP flag.
func setProcGroup(cmd *exec.Cmd) {
cmd.SysProcAttr = &syscall.SysProcAttr{
CreationFlags: syscall.CREATE_NEW_PROCESS_GROUP,
}
}
// setAgentProcGroup configures long-running agent processes for process-tree
// cleanup. On Windows, agents start suspended so installProcessLifecycle can
// bind them to a kill-on-close Job Object before they can spawn descendants.
func setAgentProcGroup(cmd *exec.Cmd) {
setProcGroup(cmd)
cmd.SysProcAttr.CreationFlags |= windows.CREATE_SUSPENDED
}
type processLifecycleHandle struct {
job winproc.KillOnCloseJob
}
func installProcessLifecycle(cmd *exec.Cmd) (processLifecycleHandle, error) {
job, err := winproc.InstallKillOnCloseJobForSuspendedCommand(cmd)
if err != nil {
return processLifecycleHandle{}, err
}
return processLifecycleHandle{job: job}, nil
}
func releaseProcessLifecycle(lifecycle processLifecycleHandle) {
_ = lifecycle.job.Close()
}
// killProcessGroup kills the entire process tree for the given PID.
// On Windows, we use taskkill with /T flag to kill the process tree.
func killProcessGroup(pid int) error {
// taskkill /F /T /PID <pid>
// /F = force kill
// /T = kill child processes (tree)
// /PID = process ID
kill := exec.Command("taskkill", "/F", "/T", "/PID", fmt.Sprintf("%d", pid))
return kill.Run()
return runProcessTaskkill("/F", "/T", "/PID", fmt.Sprintf("%d", pid))
}
// terminateProcessGroup sends a graceful shutdown signal to the process tree.
// Without /F, taskkill sends WM_CLOSE to console and windowed apps, which is
// the closest Windows equivalent of SIGTERM.
func terminateProcessGroup(pid int) error {
kill := exec.Command("taskkill", "/T", "/PID", fmt.Sprintf("%d", pid))
return kill.Run()
return runProcessTaskkill("/T", "/PID", fmt.Sprintf("%d", pid))
}
func processGroupAlive(_ int) bool {
// taskkill /T is the authoritative process-tree operation on Windows.
// There is no Unix-style process group to poll here.
return false
}
func isProcessGroupMissing(err error) bool {
return errors.Is(err, syscall.ESRCH)
}
func runProcessTaskkill(args ...string) error {
return winproc.RunTaskkill(args...)
}
@@ -0,0 +1,104 @@
//go:build windows
package process
import (
"fmt"
"os"
"os/exec"
"path/filepath"
"strconv"
"strings"
"syscall"
"testing"
"time"
"github.com/stretchr/testify/require"
"golang.org/x/sys/windows"
)
func TestWindowsSetProcGroupDoesNotSuspendSharedHelpers(t *testing.T) {
cmd := exec.Command("kandev")
setProcGroup(cmd)
require.NotNil(t, cmd.SysProcAttr)
require.NotZero(t, cmd.SysProcAttr.CreationFlags&syscall.CREATE_NEW_PROCESS_GROUP)
require.Zero(t, cmd.SysProcAttr.CreationFlags&windows.CREATE_SUSPENDED)
}
func TestWindowsSetAgentProcGroupStartsSuspended(t *testing.T) {
cmd := exec.Command("kandev")
setAgentProcGroup(cmd)
require.NotNil(t, cmd.SysProcAttr)
require.NotZero(t, cmd.SysProcAttr.CreationFlags&syscall.CREATE_NEW_PROCESS_GROUP)
require.NotZero(t, cmd.SysProcAttr.CreationFlags&windows.CREATE_SUSPENDED)
}
func TestWindowsProcessLifecycleJobKillsDescendants(t *testing.T) {
pidFile := filepath.Join(t.TempDir(), "child.pid")
cmd := fixtureCmd(fmt.Sprintf("delay-then-child %s 200 30", pidFile))
setAgentProcGroup(cmd)
require.NoError(t, cmd.Start())
parentPID := cmd.Process.Pid
waited := false
t.Cleanup(func() {
if waited {
return
}
_ = killProcessGroup(parentPID)
_ = cmd.Process.Kill()
_ = cmd.Wait()
})
lifecycle, err := installProcessLifecycle(cmd)
require.NoError(t, err)
childPID := waitForWindowsChildPID(t, pidFile, 5*time.Second)
releaseProcessLifecycle(lifecycle)
_ = cmd.Wait()
waited = true
require.Eventually(t, func() bool {
return !windowsProcessAlive(childPID)
}, 5*time.Second, 50*time.Millisecond,
"child process %d should be killed when the job handle is released", childPID)
}
func TestWindowsProcessLifecycleInstallFailsForInvalidPID(t *testing.T) {
_, err := installProcessLifecycle(&exec.Cmd{Process: &os.Process{Pid: -1}})
require.Error(t, err)
}
func waitForWindowsChildPID(t *testing.T, pidFile string, timeout time.Duration) int {
t.Helper()
deadline := time.Now().Add(timeout)
for {
raw, err := os.ReadFile(pidFile)
if err == nil {
pid, perr := strconv.Atoi(strings.TrimSpace(string(raw)))
if perr == nil && pid > 0 {
return pid
}
}
if time.Now().After(deadline) {
t.Fatalf("timed out waiting for fixture to write child pid to %s", pidFile)
}
time.Sleep(20 * time.Millisecond)
}
}
func windowsProcessAlive(pid int) bool {
if pid <= 0 {
return false
}
output, err := exec.Command("tasklist", "/FI", fmt.Sprintf("PID eq %d", pid), "/NH").Output()
if err != nil {
return false
}
text := strings.ToLower(string(output))
if strings.Contains(text, "no tasks") {
return false
}
return strings.Contains(text, strconv.Itoa(pid))
}
@@ -366,17 +366,39 @@ func (r *ProcessRunner) Stop(ctx context.Context, req StopProcessRequest) error
// Attempt graceful shutdown, then escalate to force-kill
if proc.cmd != nil && proc.cmd.Process != nil {
pid := proc.cmd.Process.Pid
info := proc.snapshot(false)
r.logger.Debug("workspace process stop requested",
zap.String("process_id", req.ProcessID),
zap.Int("pid", pid),
zap.String("session_id", info.SessionID),
zap.String("kind", string(info.Kind)),
zap.String("script_name", info.ScriptName),
zap.String("command", info.Command),
zap.String("working_dir", info.WorkingDir))
// Phase 1: Graceful shutdown (SIGTERM on Unix, interrupt on Windows)
r.logger.Debug("workspace process interrupt requested",
zap.String("process_id", req.ProcessID),
zap.Int("pid", pid),
zap.String("signal", fmt.Sprint(os.Interrupt)))
_ = proc.cmd.Process.Signal(os.Interrupt)
// Wait for graceful exit (2 seconds) or context cancellation
select {
case <-ctx.Done():
// Context cancelled - force kill immediately
r.logger.Debug("workspace process group SIGKILL requested",
zap.String("process_id", req.ProcessID),
zap.Int("pgid", pid),
zap.String("reason", "context_canceled"),
zap.Error(ctx.Err()))
_ = killProcessGroup(pid)
case <-time.After(2 * time.Second):
// Phase 2: Grace period expired - force kill entire process tree
r.logger.Debug("workspace process group SIGKILL requested",
zap.String("process_id", req.ProcessID),
zap.Int("pgid", pid),
zap.String("reason", "grace_expired"))
_ = killProcessGroup(pid)
}
}
@@ -398,6 +420,8 @@ func (r *ProcessRunner) StopAll(ctx context.Context) error {
}
r.mu.RUnlock()
r.logger.Debug("workspace process stop all requested", zap.Int("processes", len(ids)))
var errs []error
for _, id := range ids {
if err := r.Stop(ctx, StopProcessRequest{ProcessID: id}); err != nil {
@@ -7,6 +7,9 @@ import (
"time"
"github.com/kandev/kandev/internal/common/logger"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"go.uber.org/zap/zaptest/observer"
)
func newTestLogger(t *testing.T) *logger.Logger {
@@ -20,6 +23,25 @@ func newTestLogger(t *testing.T) *logger.Logger {
return log
}
func newObservedTestLogger(t *testing.T) (*logger.Logger, *observer.ObservedLogs) {
t.Helper()
core, observed := observer.New(zapcore.DebugLevel)
log, err := logger.NewFromZap(zap.New(core))
if err != nil {
t.Fatalf("failed to create observed logger: %v", err)
}
return log, observed
}
func observedLogsContain(logs *observer.ObservedLogs, message string) bool {
for _, entry := range logs.All() {
if entry.Message == message {
return true
}
}
return false
}
func TestRingBufferTrimsOldest(t *testing.T) {
buffer := newRingBuffer(10)
buffer.append(ProcessOutputChunk{Stream: "stdout", Data: "hello", Timestamp: time.Now()}) // 5
@@ -79,6 +101,55 @@ func TestProcessRunnerCapturesOutput(t *testing.T) {
t.Fatal("process output not captured in time")
}
func TestProcessRunnerStopLogsSignalAttempts(t *testing.T) {
log, observed := newObservedTestLogger(t)
runner := NewProcessRunner(nil, log, 2*1024*1024)
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
cmd, env := fixtureShellExec("sleep 30")
info, err := runner.Start(ctx, StartProcessRequest{
SessionID: "session-1",
Kind: "dev",
Command: cmd,
Env: env,
WorkingDir: "",
})
if err != nil {
t.Fatalf("failed to start process: %v", err)
}
t.Cleanup(func() {
cleanupCtx, cleanupCancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cleanupCancel()
_ = runner.Stop(cleanupCtx, StopProcessRequest{ProcessID: info.ID})
})
stopCtx, stopCancel := context.WithCancel(context.Background())
stopCancel()
if err := runner.Stop(stopCtx, StopProcessRequest{ProcessID: info.ID}); err != nil {
t.Fatalf("failed to stop process: %v", err)
}
for _, message := range []string{
"workspace process stop requested",
"workspace process interrupt requested",
"workspace process group SIGKILL requested",
} {
if !observedLogsContain(observed, message) {
t.Fatalf("expected debug log %q, got %#v", message, observed.All())
}
}
deadline := time.Now().Add(2 * time.Second)
for time.Now().Before(deadline) {
if _, ok := runner.Get(info.ID, false); !ok {
return
}
time.Sleep(20 * time.Millisecond)
}
t.Fatal("process was not removed after stop")
}
func TestStripANSI(t *testing.T) {
tests := []struct {
name string
@@ -42,6 +42,8 @@ func TestMain(m *testing.M) {
// echo <args...> — print args joined by spaces, plus newline
// cat — copy stdin to stdout until EOF
// echo-then-sleep <msg> <secs> — print msg, then sleep <secs>
// delay-then-child <pidfile> <delay-ms> <secs>
// — wait, spawn a sleeping child, write its PID, then sleep
//
// New commands can be added here as tests need them; the goal is to keep the
// surface tiny so the helper stays inspectable.
@@ -112,6 +114,61 @@ func runFixture(spec string) {
os.Exit(2)
}
time.Sleep(time.Duration(secs) * time.Second)
case "delay-then-child":
// Waits briefly before spawning the child so platform lifecycle hooks
// installed immediately after parent Start() can be applied before the
// child inherits them. Used by Windows Job Object tests.
if len(parts) != 4 {
fmt.Fprintln(os.Stderr, "fixture: delay-then-child takes 3 args")
os.Exit(2)
}
pidFile := parts[1]
delayMS, err := strconv.Atoi(parts[2])
if err != nil {
fmt.Fprintf(os.Stderr, "fixture: delay-then-child: bad delay %q\n", parts[2])
os.Exit(2)
}
secs, err := strconv.Atoi(parts[3])
if err != nil {
fmt.Fprintf(os.Stderr, "fixture: delay-then-child: bad seconds %q\n", parts[3])
os.Exit(2)
}
time.Sleep(time.Duration(delayMS) * time.Millisecond)
childCmd := exec.Command(os.Args[0])
childCmd.Env = append(os.Environ(), kandevTestFixtureEnv+"=sleep "+strconv.Itoa(secs))
if err := childCmd.Start(); err != nil {
fmt.Fprintf(os.Stderr, "fixture: delay-then-child: spawn child: %v\n", err)
os.Exit(2)
}
if err := os.WriteFile(pidFile, []byte(strconv.Itoa(childCmd.Process.Pid)), 0o600); err != nil {
fmt.Fprintf(os.Stderr, "fixture: delay-then-child: write pidfile: %v\n", err)
os.Exit(2)
}
time.Sleep(time.Duration(secs) * time.Second)
case "exit-with-child":
// Forks a sleeping child, writes its PID, then exits immediately.
// This models wrappers such as npx/node that can exit while a native
// descendant remains alive in the inherited process group.
if len(parts) != 3 {
fmt.Fprintln(os.Stderr, "fixture: exit-with-child takes 2 args")
os.Exit(2)
}
pidFile := parts[1]
secs, err := strconv.Atoi(parts[2])
if err != nil {
fmt.Fprintf(os.Stderr, "fixture: exit-with-child: bad seconds %q\n", parts[2])
os.Exit(2)
}
childCmd := exec.Command(os.Args[0])
childCmd.Env = append(os.Environ(), kandevTestFixtureEnv+"=sleep "+strconv.Itoa(secs))
if err := childCmd.Start(); err != nil {
fmt.Fprintf(os.Stderr, "fixture: exit-with-child: spawn child: %v\n", err)
os.Exit(2)
}
if err := os.WriteFile(pidFile, []byte(strconv.Itoa(childCmd.Process.Pid)), 0o600); err != nil {
fmt.Fprintf(os.Stderr, "fixture: exit-with-child: write pidfile: %v\n", err)
os.Exit(2)
}
default:
fmt.Fprintf(os.Stderr, "fixture: unknown command %q\n", parts[0])
os.Exit(2)
@@ -334,6 +334,9 @@ func (v *VscodeManager) Stop(ctx context.Context) error {
logChildProcesses(v.logger, pid)
// Phase 1: Graceful SIGTERM to the entire process group.
v.logger.Debug("code-server process group SIGTERM requested",
zap.Int("pgid", pid),
zap.String("reason", "stop_requested"))
if err := terminateProcessGroup(pid); err != nil {
v.logger.Warn("failed to send SIGTERM to code-server group",
zap.Int("pgid", pid), zap.Error(err))
@@ -353,6 +356,9 @@ func (v *VscodeManager) Stop(ctx context.Context) error {
zap.Int("pgid", pid))
}
v.logger.Debug("code-server process group SIGKILL requested",
zap.Int("pgid", pid),
zap.String("reason", "grace_expired_or_context_canceled"))
if err := killProcessGroup(pid); err != nil {
v.logger.Warn("failed to force kill code-server group",
zap.Int("pgid", pid), zap.Error(err))
@@ -0,0 +1,28 @@
//go:build !windows
package shell
import (
"errors"
"os"
"os/exec"
"syscall"
)
func configureShellProcess(_ *exec.Cmd) {}
func killShellProcessGroup(p *os.Process) error {
if p == nil {
return nil
}
if err := syscall.Kill(-p.Pid, syscall.SIGKILL); err != nil {
if errors.Is(err, syscall.ESRCH) {
return nil
}
if killErr := p.Kill(); killErr != nil && !errors.Is(killErr, os.ErrProcessDone) {
return errors.Join(err, killErr)
}
return err
}
return nil
}
@@ -0,0 +1,43 @@
//go:build windows
package shell
import (
"errors"
"fmt"
"os"
"os/exec"
"strings"
"syscall"
)
func configureShellProcess(cmd *exec.Cmd) {
cmd.SysProcAttr = &syscall.SysProcAttr{
CreationFlags: syscall.CREATE_NEW_PROCESS_GROUP,
}
}
func killShellProcessGroup(p *os.Process) error {
if p == nil {
return nil
}
if err := runShellTaskkill("/F", "/T", "/PID", fmt.Sprintf("%d", p.Pid)); err != nil {
if killErr := p.Kill(); killErr != nil && !errors.Is(killErr, os.ErrProcessDone) {
return errors.Join(err, killErr)
}
return err
}
return nil
}
func runShellTaskkill(args ...string) error {
output, err := exec.Command("taskkill", args...).CombinedOutput()
if err == nil {
return nil
}
msg := strings.TrimSpace(string(output))
if msg == "" {
return err
}
return fmt.Errorf("%w: %s", err, msg)
}
@@ -51,6 +51,7 @@ type Session struct {
// Maximum size of output buffer (16KB should be enough for recent history)
const maxOutputBufferSize = 16 * 1024
const shellStopGrace = 250 * time.Millisecond
// Config holds shell session configuration
type Config struct {
@@ -166,6 +167,7 @@ func (s *Session) start(cfg Config) error {
s.cmd = exec.Command(s.shell, s.shellArgs...)
s.cmd.Dir = cfg.WorkDir
s.cmd.Env = buildShellEnv(cfg.WorkDir)
configureShellProcess(s.cmd)
// Start with PTY
var err error
@@ -206,13 +208,26 @@ func (s *Session) Stop() error {
s.stopping = true // Mark as stopping to prevent respawn
s.mu.Unlock()
s.logger.Info("stopping shell session")
pid := 0
if s.cmd != nil && s.cmd.Process != nil {
pid = s.cmd.Process.Pid
}
s.logger.Info("stopping shell session",
zap.String("shell", s.shell),
zap.String("cwd", s.workDir),
zap.Int("pid", pid),
zap.Duration("grace", shellStopGrace))
s.logger.Debug("shell session stop requested",
zap.Int("pid", pid),
zap.String("shell", s.shell),
zap.String("cwd", s.workDir))
// Signal stop
close(s.stopCh)
// Close PTY (sends SIGHUP to process on Unix)
if s.pty != nil {
s.logger.Debug("shell session PTY close requested", zap.Int("pid", pid))
_ = s.pty.Close()
}
@@ -220,16 +235,32 @@ func (s *Session) Stop() error {
select {
case <-s.doneCh:
s.logger.Info("shell session stopped gracefully")
case <-time.After(5 * time.Second):
s.logger.Warn("shell session stop timeout, force killing")
if s.cmd != nil && s.cmd.Process != nil {
_ = s.cmd.Process.Kill()
}
s.cleanupShellProcessGroup(pid, "leader_exited")
case <-time.After(shellStopGrace):
s.logger.Warn("shell session stop timeout, force killing",
zap.Int("pid", pid),
zap.Duration("grace", shellStopGrace))
s.cleanupShellProcessGroup(pid, "stop_timeout")
}
return nil
}
func (s *Session) cleanupShellProcessGroup(pid int, reason string) {
if s.cmd == nil || s.cmd.Process == nil {
return
}
s.logger.Debug("shell session process group cleanup requested",
zap.Int("pid", pid),
zap.String("reason", reason))
if err := killShellProcessGroup(s.cmd.Process); err != nil {
s.logger.Debug("shell session process group cleanup failed",
zap.Int("pid", pid),
zap.String("reason", reason),
zap.Error(err))
}
}
// Write sends input to the shell
func (s *Session) Write(data []byte) (int, error) {
s.mu.RLock()
@@ -0,0 +1,101 @@
//go:build !windows
package shell
import (
"fmt"
"os"
"os/exec"
"path/filepath"
"strconv"
"strings"
"syscall"
"testing"
"time"
"github.com/stretchr/testify/require"
)
func TestStopKillsShellProcessGroupOnTimeout(t *testing.T) {
sh, err := exec.LookPath("sh")
require.NoError(t, err)
pidFile := filepath.Join(t.TempDir(), "child.pid")
script := fmt.Sprintf("sleep 30 & echo $! > %s; trap '' HUP TERM; wait", shellQuote(pidFile))
session, err := NewSession(Config{
WorkDir: t.TempDir(),
ShellCommand: sh,
ShellArgs: []string{"-c", script},
}, newTestLogger())
require.NoError(t, err)
childPID := waitForShellChildPID(t, pidFile)
require.NoError(t, session.Stop())
require.Eventually(t, func() bool {
return !shellProcessAlive(childPID)
}, 5*time.Second, 50*time.Millisecond, "shell child process %d should be killed", childPID)
}
func TestStopKillsShellProcessGroupAfterLeaderExits(t *testing.T) {
sh, err := exec.LookPath("sh")
require.NoError(t, err)
pidFile := filepath.Join(t.TempDir(), "child.pid")
script := fmt.Sprintf("trap 'exit 0' HUP TERM; (trap '' HUP TERM; sleep 30) & echo $! > %s; wait", shellQuote(pidFile))
session, err := NewSession(Config{
WorkDir: t.TempDir(),
ShellCommand: sh,
ShellArgs: []string{"-c", script},
}, newTestLogger())
require.NoError(t, err)
childPID := waitForShellChildPID(t, pidFile)
require.NoError(t, session.Stop())
require.Eventually(t, func() bool {
return !shellProcessAlive(childPID)
}, 5*time.Second, 50*time.Millisecond, "shell child process %d should be killed after shell leader exits", childPID)
}
func waitForShellChildPID(t *testing.T, pidFile string) int {
t.Helper()
deadline := time.Now().Add(5 * time.Second)
for time.Now().Before(deadline) {
raw, err := os.ReadFile(pidFile)
if err == nil {
pid, perr := strconv.Atoi(strings.TrimSpace(string(raw)))
if perr == nil && pid > 0 {
return pid
}
}
time.Sleep(20 * time.Millisecond)
}
t.Fatalf("timed out waiting for shell child pid file %s", pidFile)
return 0
}
func shellProcessAlive(pid int) bool {
if pid <= 0 {
return false
}
if _, err := os.Stat(fmt.Sprintf("/proc/%d", pid)); err == nil {
raw, err := os.ReadFile(fmt.Sprintf("/proc/%d/stat", pid))
if err != nil {
return false
}
stat := string(raw)
idx := strings.LastIndex(stat, ") ")
if idx == -1 || idx+2 >= len(stat) {
return false
}
return stat[idx+2] != 'Z'
}
proc, err := os.FindProcess(pid)
if err != nil {
return false
}
return proc.Signal(syscall.Signal(0)) == nil
}
func shellQuote(s string) string {
return "'" + strings.ReplaceAll(s, "'", "'\\''") + "'"
}
@@ -206,6 +206,26 @@ func TestSessionStop(t *testing.T) {
}
}
func TestSessionStopTimeoutIsShortForShutdown(t *testing.T) {
log := newTestLogger()
session := &Session{
logger: log,
running: true,
stopCh: make(chan struct{}),
doneCh: make(chan struct{}),
}
start := time.Now()
if err := session.Stop(); err != nil {
t.Errorf("Stop failed: %v", err)
}
elapsed := time.Since(start)
if elapsed > 700*time.Millisecond {
t.Fatalf("Stop took %s, want less than 700ms for shutdown fallback", elapsed)
}
}
// TestSessionWrite tests writing to the shell
func TestSessionWrite(t *testing.T) {
if runtime.GOOS == "windows" {
@@ -0,0 +1,165 @@
package utility
import (
"context"
"os"
"os/exec"
"time"
"go.uber.org/zap"
)
func configureACPCommand(cmd *exec.Cmd, log *zap.Logger) {
log = acpCommandLogger(log)
setACPCommandProcAttr(cmd)
cmd.Cancel = func() error {
if cmd.Process == nil {
return os.ErrProcessDone
}
log.Debug("ACP command process group SIGTERM requested",
zap.Int("pgid", cmd.Process.Pid),
zap.String("reason", "context_cancel"))
if err := terminateACPProcessGroup(cmd.Process.Pid); err != nil {
if acpProcessGroupMissing(err) {
return os.ErrProcessDone
}
log.Debug("ACP command process group SIGTERM failed",
zap.Int("pgid", cmd.Process.Pid),
zap.String("reason", "context_cancel"),
zap.Error(err))
return err
}
return nil
}
}
func cleanupACPCommand(ctx context.Context, cmd *exec.Cmd, lifecycle acpCommandLifecycleHandle, log *zap.Logger) {
log = acpCommandLogger(log)
defer releaseACPCommandLifecycle(lifecycle)
if cmd == nil {
log.Debug("ACP command cleanup skipped", zap.String("reason", "nil_command"))
return
}
if cmd.Process == nil {
log.Debug("ACP command cleanup skipped", zap.String("reason", "nil_process"))
return
}
pid := cmd.Process.Pid
cleanupCtx, cancel := context.WithTimeout(
context.WithoutCancel(ctx),
2*acpCommandTerminateGrace+2*acpCommandForceKillGrace,
)
defer cancel()
log.Debug("ACP command cleanup requested",
zap.Int("pid", pid),
zap.String("path", cmd.Path),
zap.Strings("args", cmd.Args))
log.Debug("ACP command process group SIGTERM requested",
zap.Int("pgid", pid),
zap.String("reason", "cleanup"))
if err := terminateACPProcessGroup(pid); err != nil && !acpProcessGroupMissing(err) {
log.Debug("ACP command process group SIGTERM failed",
zap.Int("pgid", pid),
zap.String("reason", "cleanup"),
zap.Error(err))
}
waitCh := make(chan error, 1)
go func() {
waitCh <- cmd.Wait()
}()
if waitForACPCommand(cleanupCtx, waitCh, acpCommandTerminateGrace) {
log.Debug("ACP command exited after SIGTERM", zap.Int("pid", pid))
reapACPProcessGroup(cleanupCtx, pid, log)
return
}
log.Debug("ACP command process group SIGKILL requested",
zap.Int("pgid", pid),
zap.String("reason", "cleanup_timeout"))
if err := killACPProcessGroup(pid); err != nil && !acpProcessGroupMissing(err) {
log.Debug("ACP command process group SIGKILL failed; killing process",
zap.Int("pgid", pid),
zap.Int("pid", pid),
zap.Error(err))
log.Debug("ACP command process SIGKILL requested",
zap.Int("pid", pid),
zap.String("reason", "process_group_kill_failed"))
_ = cmd.Process.Kill()
}
_ = waitForACPCommand(cleanupCtx, waitCh, acpCommandForceKillGrace)
reapACPProcessGroup(cleanupCtx, pid, log)
}
func waitForACPCommand(ctx context.Context, waitCh <-chan error, timeout time.Duration) bool {
timer := time.NewTimer(timeout)
defer timer.Stop()
select {
case <-waitCh:
return true
case <-ctx.Done():
return false
case <-timer.C:
return false
}
}
func reapACPProcessGroup(ctx context.Context, pid int, log *zap.Logger) {
log = acpCommandLogger(log)
if !acpProcessGroupAlive(pid) {
return
}
log.Debug("ACP command process group still alive; SIGTERM requested",
zap.Int("pgid", pid),
zap.String("reason", "reap_descendants"))
if err := terminateACPProcessGroup(pid); err != nil && !acpProcessGroupMissing(err) {
log.Debug("ACP command process group SIGTERM failed",
zap.Int("pgid", pid),
zap.String("reason", "reap_descendants"),
zap.Error(err))
}
waitForACPProcessGroupExit(ctx, pid, acpCommandTerminateGrace)
if !acpProcessGroupAlive(pid) {
return
}
log.Debug("ACP command process group SIGKILL requested",
zap.Int("pgid", pid),
zap.String("reason", "reap_descendants_timeout"))
if err := killACPProcessGroup(pid); err != nil && !acpProcessGroupMissing(err) {
log.Debug("ACP command process group SIGKILL failed",
zap.Int("pgid", pid),
zap.String("reason", "reap_descendants_timeout"),
zap.Error(err))
}
waitForACPProcessGroupExit(ctx, pid, acpCommandForceKillGrace)
}
func acpCommandLogger(log *zap.Logger) *zap.Logger {
if log == nil {
return zap.NewNop()
}
return log
}
func waitForACPProcessGroupExit(ctx context.Context, pid int, timeout time.Duration) bool {
if !acpProcessGroupAlive(pid) {
return true
}
timer := time.NewTimer(timeout)
defer timer.Stop()
ticker := time.NewTicker(acpCommandPollInterval)
defer ticker.Stop()
for {
select {
case <-ctx.Done():
return false
case <-timer.C:
return false
case <-ticker.C:
if !acpProcessGroupAlive(pid) {
return true
}
}
}
}
@@ -0,0 +1,44 @@
//go:build linux
package utility
import (
"errors"
"os/exec"
"syscall"
)
func setACPCommandProcAttr(cmd *exec.Cmd) {
cmd.SysProcAttr = &syscall.SysProcAttr{
Setpgid: true,
Pdeathsig: syscall.SIGTERM,
}
}
type acpCommandLifecycleHandle struct{}
func installACPCommandLifecycle(_ *exec.Cmd) (acpCommandLifecycleHandle, error) {
return acpCommandLifecycleHandle{}, nil
}
func releaseACPCommandLifecycle(_ acpCommandLifecycleHandle) {}
func terminateACPProcessGroup(pid int) error {
return syscall.Kill(-pid, syscall.SIGTERM)
}
func killACPProcessGroup(pid int) error {
return syscall.Kill(-pid, syscall.SIGKILL)
}
func acpProcessGroupAlive(pid int) bool {
if pid <= 0 {
return false
}
err := syscall.Kill(-pid, 0)
return err == nil || errors.Is(err, syscall.EPERM)
}
func acpProcessGroupMissing(err error) bool {
return errors.Is(err, syscall.ESRCH)
}
@@ -0,0 +1,41 @@
//go:build unix && !linux
package utility
import (
"errors"
"os/exec"
"syscall"
)
func setACPCommandProcAttr(cmd *exec.Cmd) {
cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
}
type acpCommandLifecycleHandle struct{}
func installACPCommandLifecycle(_ *exec.Cmd) (acpCommandLifecycleHandle, error) {
return acpCommandLifecycleHandle{}, nil
}
func releaseACPCommandLifecycle(_ acpCommandLifecycleHandle) {}
func terminateACPProcessGroup(pid int) error {
return syscall.Kill(-pid, syscall.SIGTERM)
}
func killACPProcessGroup(pid int) error {
return syscall.Kill(-pid, syscall.SIGKILL)
}
func acpProcessGroupAlive(pid int) bool {
if pid <= 0 {
return false
}
err := syscall.Kill(-pid, 0)
return err == nil || errors.Is(err, syscall.EPERM)
}
func acpProcessGroupMissing(err error) bool {
return errors.Is(err, syscall.ESRCH)
}
@@ -0,0 +1,76 @@
//go:build windows
package utility
import (
"fmt"
"os/exec"
"strconv"
"strings"
"syscall"
"github.com/kandev/kandev/internal/agentctl/server/winproc"
"golang.org/x/sys/windows"
)
// setACPCommandProcAttr starts ACP utility commands suspended so the cleanup
// lifecycle can attach a kill-on-close Job Object before the command runs.
func setACPCommandProcAttr(cmd *exec.Cmd) {
cmd.SysProcAttr = &syscall.SysProcAttr{
CreationFlags: syscall.CREATE_NEW_PROCESS_GROUP | windows.CREATE_SUSPENDED,
}
}
type acpCommandLifecycleHandle struct {
job winproc.KillOnCloseJob
}
func installACPCommandLifecycle(cmd *exec.Cmd) (acpCommandLifecycleHandle, error) {
job, err := winproc.InstallKillOnCloseJobForSuspendedCommand(cmd)
if err != nil {
return acpCommandLifecycleHandle{}, err
}
return acpCommandLifecycleHandle{job: job}, nil
}
func releaseACPCommandLifecycle(lifecycle acpCommandLifecycleHandle) {
_ = lifecycle.job.Close()
}
func terminateACPProcessGroup(pid int) error {
return runTaskkill("/T", "/PID", fmt.Sprintf("%d", pid))
}
func killACPProcessGroup(pid int) error {
return runTaskkill("/F", "/T", "/PID", fmt.Sprintf("%d", pid))
}
func runTaskkill(args ...string) error {
return winproc.RunTaskkill(args...)
}
func acpProcessGroupAlive(pid int) bool {
if pid <= 0 {
return false
}
output, err := exec.Command("tasklist", "/FI", fmt.Sprintf("PID eq %d", pid), "/NH").Output()
if err != nil {
return false
}
text := strings.ToLower(string(output))
if strings.Contains(text, "no tasks") {
return false
}
return strings.Contains(text, strconv.Itoa(pid))
}
func acpProcessGroupMissing(err error) bool {
if err == nil {
return false
}
msg := strings.ToLower(err.Error())
return strings.Contains(msg, "not found") ||
strings.Contains(msg, "not be found") ||
strings.Contains(msg, "no running instance") ||
strings.Contains(msg, "not running")
}
@@ -0,0 +1,151 @@
//go:build windows
package utility
import (
"fmt"
"io"
"os"
"os/exec"
"path/filepath"
"strconv"
"strings"
"testing"
"time"
"github.com/stretchr/testify/require"
)
const acpCommandTestFixtureEnv = "KANDEV_ACP_COMMAND_TEST_FIXTURE"
func TestMain(m *testing.M) {
if spec := os.Getenv(acpCommandTestFixtureEnv); spec != "" {
runACPCommandTestFixture(spec)
return
}
os.Exit(m.Run())
}
func TestWindowsACPCommandLifecycleJobKillsDescendants(t *testing.T) {
pidFile := filepath.Join(t.TempDir(), "child.pid")
cmd := exec.Command(os.Args[0])
cmd.Env = append(os.Environ(), acpCommandTestFixtureEnv+"="+fmt.Sprintf("delay-then-child %s 200 30", pidFile))
setACPCommandProcAttr(cmd)
require.NoError(t, cmd.Start())
parentPID := cmd.Process.Pid
waited := false
t.Cleanup(func() {
if waited {
return
}
_ = killACPProcessGroup(parentPID)
_ = cmd.Process.Kill()
_ = cmd.Wait()
})
lifecycle, err := installACPCommandLifecycle(cmd)
require.NoError(t, err)
childPID := waitForACPCommandChildPID(t, pidFile, 5*time.Second)
releaseACPCommandLifecycle(lifecycle)
_ = cmd.Wait()
waited = true
require.Eventually(t, func() bool {
return !acpCommandWindowsProcessAlive(childPID)
}, 5*time.Second, 50*time.Millisecond,
"child process %d should be killed when the job handle is released", childPID)
}
func TestWindowsACPCommandLifecycleInstallFailsForInvalidPID(t *testing.T) {
_, err := installACPCommandLifecycle(&exec.Cmd{Process: &os.Process{Pid: -1}})
require.Error(t, err)
}
func runACPCommandTestFixture(spec string) {
parts := strings.Fields(spec)
if len(parts) == 0 {
fmt.Fprintln(os.Stderr, "fixture: empty spec")
os.Exit(2)
}
switch parts[0] {
case "sleep":
if len(parts) != 2 {
fmt.Fprintln(os.Stderr, "fixture: sleep takes 1 arg")
os.Exit(2)
}
secs, err := strconv.Atoi(parts[1])
if err != nil {
fmt.Fprintf(os.Stderr, "fixture: sleep: bad seconds %q\n", parts[1])
os.Exit(2)
}
time.Sleep(time.Duration(secs) * time.Second)
case "cat":
_, _ = io.Copy(os.Stdout, os.Stdin)
case "delay-then-child":
if len(parts) != 4 {
fmt.Fprintln(os.Stderr, "fixture: delay-then-child takes 3 args")
os.Exit(2)
}
pidFile := parts[1]
delayMS, err := strconv.Atoi(parts[2])
if err != nil {
fmt.Fprintf(os.Stderr, "fixture: delay-then-child: bad delay %q\n", parts[2])
os.Exit(2)
}
secs, err := strconv.Atoi(parts[3])
if err != nil {
fmt.Fprintf(os.Stderr, "fixture: delay-then-child: bad seconds %q\n", parts[3])
os.Exit(2)
}
time.Sleep(time.Duration(delayMS) * time.Millisecond)
childCmd := exec.Command(os.Args[0])
childCmd.Env = append(os.Environ(), acpCommandTestFixtureEnv+"=sleep "+strconv.Itoa(secs))
if err := childCmd.Start(); err != nil {
fmt.Fprintf(os.Stderr, "fixture: delay-then-child: spawn child: %v\n", err)
os.Exit(2)
}
if err := os.WriteFile(pidFile, []byte(strconv.Itoa(childCmd.Process.Pid)), 0o600); err != nil {
fmt.Fprintf(os.Stderr, "fixture: delay-then-child: write pidfile: %v\n", err)
os.Exit(2)
}
time.Sleep(time.Duration(secs) * time.Second)
default:
fmt.Fprintf(os.Stderr, "fixture: unknown command %q\n", parts[0])
os.Exit(2)
}
os.Exit(0)
}
func waitForACPCommandChildPID(t *testing.T, pidFile string, timeout time.Duration) int {
t.Helper()
deadline := time.Now().Add(timeout)
for {
raw, err := os.ReadFile(pidFile)
if err == nil {
pid, perr := strconv.Atoi(strings.TrimSpace(string(raw)))
if perr == nil && pid > 0 {
return pid
}
}
if time.Now().After(deadline) {
t.Fatalf("timed out waiting for fixture to write child pid to %s", pidFile)
}
time.Sleep(20 * time.Millisecond)
}
}
func acpCommandWindowsProcessAlive(pid int) bool {
if pid <= 0 {
return false
}
output, err := exec.Command("tasklist", "/FI", fmt.Sprintf("PID eq %d", pid), "/NH").Output()
if err != nil {
return false
}
text := strings.ToLower(string(output))
if strings.Contains(text, "no tasks") {
return false
}
return strings.Contains(text, strconv.Itoa(pid))
}
@@ -23,6 +23,10 @@ import (
const (
openCodeCommand = "opencode"
openCodeACPSubcommand = "acp"
acpCommandTerminateGrace = 250 * time.Millisecond
acpCommandForceKillGrace = 500 * time.Millisecond
acpCommandPollInterval = 25 * time.Millisecond
)
// ACPInferenceExecutor executes one-shot prompts using the ACP protocol.
@@ -72,6 +76,7 @@ func (e *ACPInferenceExecutor) Execute(ctx context.Context, req *PromptRequest)
//nolint:gosec // resolvedCmd is from a hard-coded allow-list; args[1:] are CLI flags
cmd := exec.CommandContext(ctx, resolvedCmd, args[1:]...)
cmd.Dir = workDir
configureACPCommand(cmd, e.logger)
stdin, err := cmd.StdinPipe()
if err != nil {
@@ -86,12 +91,13 @@ func (e *ACPInferenceExecutor) Execute(ctx context.Context, req *PromptRequest)
if err := cmd.Start(); err != nil {
return &PromptResponse{Success: false, Error: fmt.Sprintf("start: %v", err)}, nil
}
lifecycle, err := installACPCommandLifecycle(cmd)
if err != nil {
e.logger.Warn("failed to install ACP command lifecycle; falling back to process-tree cleanup",
zap.Error(err))
}
// Ensure process cleanup
defer func() {
_ = cmd.Process.Kill()
_ = cmd.Wait()
}()
defer cleanupACPCommand(ctx, cmd, lifecycle, e.logger)
// Execute ACP protocol
mcpServers, dropped := toACPMcpServers(req.MCPServers)
@@ -316,6 +322,7 @@ func (e *ACPInferenceExecutor) Probe(ctx context.Context, req *ProbeRequest) (*P
//nolint:gosec // resolvedCmd is from a hard-coded allow-list; args[1:] are CLI flags
cmd := exec.CommandContext(ctx, resolvedCmd, args[1:]...)
cmd.Dir = workDir
configureACPCommand(cmd, e.logger)
stdin, err := cmd.StdinPipe()
if err != nil {
@@ -328,10 +335,12 @@ func (e *ACPInferenceExecutor) Probe(ctx context.Context, req *ProbeRequest) (*P
if err := cmd.Start(); err != nil {
return &ProbeResponse{Success: false, Error: fmt.Sprintf("start: %v", err)}, nil
}
defer func() {
_ = cmd.Process.Kill()
_ = cmd.Wait()
}()
lifecycle, err := installACPCommandLifecycle(cmd)
if err != nil {
e.logger.Warn("failed to install ACP command lifecycle; falling back to process-tree cleanup",
zap.Error(err))
}
defer cleanupACPCommand(ctx, cmd, lifecycle, e.logger)
resp, err := e.probeACPSession(ctx, stdin, stdout, workDir)
if err != nil {
@@ -0,0 +1,208 @@
//go:build linux
package utility
import (
"context"
"fmt"
"os"
"os/exec"
"path/filepath"
"strconv"
"strings"
"syscall"
"testing"
"time"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"go.uber.org/zap/zaptest/observer"
)
func TestProbeCleansUpDescendantProcessOnTimeout(t *testing.T) {
tmp := t.TempDir()
marker := filepath.Join(tmp, "child")
installLeakyMockAgent(t, tmp, marker)
t.Setenv("PATH", tmp+string(os.PathListSeparator)+os.Getenv("PATH"))
t.Setenv("ACP_LEAK_MARKER", marker)
ctx, cancel := context.WithTimeout(context.Background(), 150*time.Millisecond)
defer cancel()
core, observed := observer.New(zapcore.DebugLevel)
executor := NewACPInferenceExecutor(zap.New(core))
resp, err := executor.Probe(ctx, &ProbeRequest{
AgentID: "mock-agent",
InferenceConfig: &InferenceConfigDTO{
Command: []string{"mock-agent"},
WorkDir: tmp,
},
})
if err != nil {
t.Fatalf("Probe returned error: %v", err)
}
if resp.Success {
t.Fatalf("Probe unexpectedly succeeded")
}
pid := readPID(t, marker+".pid")
t.Cleanup(func() {
if processRunning(pid) {
_ = syscall.Kill(pid, syscall.SIGKILL)
}
})
waitUntil(t, 2*time.Second, func() bool {
return !processRunning(pid)
}, "descendant process %d was still running after Probe returned", pid)
for _, message := range []string{
"ACP command process group SIGTERM requested",
"ACP command process group SIGKILL requested",
} {
if !zapLogsContain(observed, message) {
t.Fatalf("expected debug log %q, got %#v", message, observed.All())
}
}
}
func TestWaitForACPProcessGroupExitHonorsContextCancellation(t *testing.T) {
cmd := exec.Command("sleep", "30")
setACPCommandProcAttr(cmd)
if err := cmd.Start(); err != nil {
t.Fatalf("start sleep: %v", err)
}
pid := cmd.Process.Pid
t.Cleanup(func() {
_ = killACPProcessGroup(pid)
done := make(chan error, 1)
go func() { done <- cmd.Wait() }()
select {
case <-done:
case <-time.After(time.Second):
_ = cmd.Process.Kill()
}
})
ctx, cancel := context.WithCancel(context.Background())
cancel()
start := time.Now()
if waitForACPProcessGroupExit(ctx, pid, 2*time.Second) {
t.Fatal("wait unexpectedly reported process group exit")
}
if elapsed := time.Since(start); elapsed > 100*time.Millisecond {
t.Fatalf("wait ignored canceled context; elapsed=%s", elapsed)
}
}
func TestCleanupACPCommandWaitsAfterRequestContextCancellation(t *testing.T) {
cmd := exec.Command("sleep", "30")
setACPCommandProcAttr(cmd)
if err := cmd.Start(); err != nil {
t.Fatalf("start sleep: %v", err)
}
pid := cmd.Process.Pid
waited := false
t.Cleanup(func() {
if waited {
return
}
_ = killACPProcessGroup(pid)
_ = cmd.Wait()
})
ctx, cancel := context.WithCancel(context.Background())
cancel()
core, observed := observer.New(zapcore.DebugLevel)
cleanupACPCommand(ctx, cmd, acpCommandLifecycleHandle{}, zap.New(core))
waited = true
if !zapLogsContain(observed, "ACP command exited after SIGTERM") {
t.Fatalf("cleanup did not wait for SIGTERM exit after canceled context; logs=%#v", observed.All())
}
if processRunning(pid) {
t.Fatalf("process %d still running after cleanup", pid)
}
}
func zapLogsContain(logs *observer.ObservedLogs, message string) bool {
for _, entry := range logs.All() {
if entry.Message == message {
return true
}
}
return false
}
func installLeakyMockAgent(t *testing.T, dir, marker string) {
t.Helper()
agentPath := filepath.Join(dir, "mock-agent")
childPath := filepath.Join(dir, "mock-agent-child")
writeExecutable(t, agentPath, fmt.Sprintf(`#!/bin/sh
"%s" "$ACP_LEAK_MARKER" &
echo "$!" > "$ACP_LEAK_MARKER.pid"
sleep 30
`, childPath))
writeExecutable(t, childPath, `#!/bin/sh
trap '' TERM INT HUP
touch "$1.ready"
sleep 30
`)
t.Setenv("ACP_LEAK_MARKER", marker)
}
func writeExecutable(t *testing.T, path, content string) {
t.Helper()
if err := os.WriteFile(path, []byte(content), 0o755); err != nil {
t.Fatalf("write executable %s: %v", path, err)
}
}
func readPID(t *testing.T, path string) int {
t.Helper()
var raw []byte
waitUntil(t, 2*time.Second, func() bool {
var err error
raw, err = os.ReadFile(path)
return err == nil
}, "pid file %s was not written", path)
pid, err := strconv.Atoi(strings.TrimSpace(string(raw)))
if err != nil {
t.Fatalf("parse pid file %s: %v", path, err)
}
return pid
}
func waitUntil(t *testing.T, timeout time.Duration, condition func() bool, format string, args ...any) {
t.Helper()
deadline := time.Now().Add(timeout)
for time.Now().Before(deadline) {
if condition() {
return
}
time.Sleep(20 * time.Millisecond)
}
if condition() {
return
}
t.Fatalf(format, args...)
}
func processRunning(pid int) bool {
if pid <= 0 {
return false
}
if err := syscall.Kill(pid, 0); err != nil {
return false
}
stat, err := os.ReadFile(fmt.Sprintf("/proc/%d/stat", pid))
if err != nil {
return true
}
return !strings.Contains(string(stat), ") Z ")
}
@@ -0,0 +1,3 @@
//go:build !windows
package winproc
@@ -0,0 +1,190 @@
//go:build windows
package winproc
import (
"errors"
"fmt"
"os/exec"
"strings"
"syscall"
"unsafe"
"golang.org/x/sys/windows"
)
// KillOnCloseJob owns a Windows Job Object configured to terminate all assigned
// processes when the handle is closed.
type KillOnCloseJob struct {
handle windows.Handle
}
// InstallKillOnCloseJobForSuspendedCommand assigns a suspended child process to
// a kill-on-close Job Object before resuming it. If job setup fails, the child
// is still resumed so callers can fall back to explicit taskkill cleanup.
func InstallKillOnCloseJobForSuspendedCommand(cmd *exec.Cmd) (KillOnCloseJob, error) {
if cmd == nil || cmd.Process == nil {
return KillOnCloseJob{}, fmt.Errorf("process not started")
}
return InstallKillOnCloseJobForSuspendedProcess(cmd.Process.Pid)
}
// InstallKillOnCloseJobForCommand assigns an already-running child process to
// a kill-on-close Job Object. Use the suspended variant when the child must not
// execute before job assignment.
func InstallKillOnCloseJobForCommand(cmd *exec.Cmd) (KillOnCloseJob, error) {
if cmd == nil || cmd.Process == nil {
return KillOnCloseJob{}, fmt.Errorf("process not started")
}
return InstallKillOnCloseJobForProcess(cmd.Process.Pid)
}
func InstallKillOnCloseJobForProcess(pid int) (KillOnCloseJob, error) {
job, err := createKillOnCloseJob()
if err != nil {
return KillOnCloseJob{}, err
}
if err := assignProcessToJob(job, pid); err != nil {
_ = windows.CloseHandle(job)
return KillOnCloseJob{}, err
}
return KillOnCloseJob{handle: job}, nil
}
func InstallKillOnCloseJobForSuspendedProcess(pid int) (KillOnCloseJob, error) {
job, err := createKillOnCloseJob()
if err != nil {
return KillOnCloseJob{}, errors.Join(err, ResumeSuspendedProcess(pid))
}
if err := assignProcessToJob(job, pid); err != nil {
_ = windows.CloseHandle(job)
return KillOnCloseJob{}, errors.Join(
err,
ResumeSuspendedProcess(pid),
)
}
if err := ResumeSuspendedProcess(pid); err != nil {
_ = windows.CloseHandle(job)
return KillOnCloseJob{}, err
}
return KillOnCloseJob{handle: job}, nil
}
func (j KillOnCloseJob) Close() error {
if j.handle == 0 {
return nil
}
return windows.CloseHandle(j.handle)
}
func (j KillOnCloseJob) RawHandle() uintptr {
return uintptr(j.handle)
}
func createKillOnCloseJob() (windows.Handle, error) {
job, err := windows.CreateJobObject(nil, nil)
if err != nil {
return 0, fmt.Errorf("CreateJobObject: %w", err)
}
info := windows.JOBOBJECT_EXTENDED_LIMIT_INFORMATION{
BasicLimitInformation: windows.JOBOBJECT_BASIC_LIMIT_INFORMATION{
LimitFlags: windows.JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE,
},
}
if _, err := windows.SetInformationJobObject(
job,
windows.JobObjectExtendedLimitInformation,
uintptr(unsafe.Pointer(&info)),
uint32(unsafe.Sizeof(info)),
); err != nil {
_ = windows.CloseHandle(job)
return 0, fmt.Errorf("SetInformationJobObject: %w", err)
}
return job, nil
}
func assignProcessToJob(job windows.Handle, pid int) error {
procHandle, err := windows.OpenProcess(
windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE,
false,
uint32(pid),
)
if err != nil {
return fmt.Errorf("OpenProcess(pid=%d): %w", pid, err)
}
defer windows.CloseHandle(procHandle)
if err := windows.AssignProcessToJobObject(job, procHandle); err != nil {
return fmt.Errorf("AssignProcessToJobObject: %w", err)
}
return nil
}
func ResumeSuspendedProcess(pid int) error {
snapshot, err := windows.CreateToolhelp32Snapshot(windows.TH32CS_SNAPTHREAD, 0)
if err != nil {
return fmt.Errorf("CreateToolhelp32Snapshot: %w", err)
}
defer windows.CloseHandle(snapshot)
entry := windows.ThreadEntry32{Size: uint32(unsafe.Sizeof(windows.ThreadEntry32{}))}
if err := windows.Thread32First(snapshot, &entry); err != nil {
return fmt.Errorf("Thread32First: %w", err)
}
resumed := 0
for {
if entry.OwnerProcessID == uint32(pid) {
if err := resumeThread(entry.ThreadID); err != nil {
return err
}
resumed++
}
if err := windows.Thread32Next(snapshot, &entry); err != nil {
if errors.Is(err, windows.ERROR_NO_MORE_FILES) {
break
}
return fmt.Errorf("Thread32Next: %w", err)
}
}
if resumed == 0 {
return fmt.Errorf("no threads found for pid %d", pid)
}
return nil
}
func resumeThread(threadID uint32) error {
thread, err := windows.OpenThread(windows.THREAD_SUSPEND_RESUME, false, threadID)
if err != nil {
return fmt.Errorf("OpenThread(thread_id=%d): %w", threadID, err)
}
defer windows.CloseHandle(thread)
if _, err := windows.ResumeThread(thread); err != nil {
return fmt.Errorf("ResumeThread(thread_id=%d): %w", threadID, err)
}
return nil
}
func RunTaskkill(args ...string) error {
output, err := exec.Command("taskkill", args...).CombinedOutput()
if err == nil {
return nil
}
msg := strings.TrimSpace(string(output))
if IsTaskkillMissing(msg) {
return syscall.ESRCH
}
if msg == "" {
return err
}
return fmt.Errorf("%w: %s", err, msg)
}
func IsTaskkillMissing(msg string) bool {
if msg == "" {
return false
}
lower := strings.ToLower(msg)
return strings.Contains(lower, "not found") ||
strings.Contains(lower, "not be found") ||
strings.Contains(lower, "no running instance")
}
@@ -0,0 +1,67 @@
//go:build windows
package winproc
import (
"os/exec"
"testing"
)
func TestIsTaskkillMissing(t *testing.T) {
tests := []struct {
name string
msg string
want bool
}{
{name: "empty", msg: "", want: false},
{name: "not found", msg: "ERROR: The process \"123\" not found.", want: true},
{name: "not be found", msg: "ERROR: The process with PID 123 could not be found.", want: true},
{name: "no running instance", msg: "ERROR: No running instance of the task.", want: true},
{name: "other error", msg: "ERROR: Access is denied.", want: false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := IsTaskkillMissing(tt.msg); got != tt.want {
t.Fatalf("IsTaskkillMissing(%q) = %v, want %v", tt.msg, got, tt.want)
}
})
}
}
func TestInstallKillOnCloseJobForSuspendedCommandRejectsNilCommand(t *testing.T) {
if _, err := InstallKillOnCloseJobForSuspendedCommand(nil); err == nil {
t.Fatal("InstallKillOnCloseJobForSuspendedCommand(nil) error = nil, want non-nil")
}
if _, err := InstallKillOnCloseJobForSuspendedCommand(&exec.Cmd{}); err == nil {
t.Fatal("InstallKillOnCloseJobForSuspendedCommand(cmd without process) error = nil, want non-nil")
}
}
func TestInstallKillOnCloseJobForCommandRejectsNilCommand(t *testing.T) {
if _, err := InstallKillOnCloseJobForCommand(nil); err == nil {
t.Fatal("InstallKillOnCloseJobForCommand(nil) error = nil, want non-nil")
}
if _, err := InstallKillOnCloseJobForCommand(&exec.Cmd{}); err == nil {
t.Fatal("InstallKillOnCloseJobForCommand(cmd without process) error = nil, want non-nil")
}
}
func TestInstallKillOnCloseJobForCommandHappyPath(t *testing.T) {
cmd := exec.Command("cmd.exe", "/c", "exit", "0")
if err := cmd.Start(); err != nil {
t.Fatalf("start child: %v", err)
}
job, err := InstallKillOnCloseJobForCommand(cmd)
if err != nil {
t.Fatalf("InstallKillOnCloseJobForCommand: %v", err)
}
if job.RawHandle() == 0 {
t.Fatal("got null job handle")
}
if err := cmd.Wait(); err != nil {
t.Fatalf("wait child: %v", err)
}
if err := job.Close(); err != nil {
t.Errorf("job.Close: %v", err)
}
}
+32 -9
View File
@@ -86,6 +86,9 @@ import (
const (
desktopHealthTokenEnv = "KANDEV_DESKTOP_HEALTH_TOKEN"
desktopHealthTokenHeader = "X-Kandev-Desktop-Health-Token"
agentShutdownTimeout = 20 * time.Second
httpShutdownTimeout = 10 * time.Second
tracingShutdownTimeout = 5 * time.Second
)
// buildSessionDataProvider constructs the session data provider function used by the WebSocket hub
@@ -1152,46 +1155,66 @@ func runGracefulShutdown(
runCleanups func(),
log *logger.Logger,
) {
log.Info("Shutting down Kandev...")
start := time.Now()
var shutdownErrs []error
log.Info("Graceful shutdown started",
zap.Int("http_timeout_seconds", int(httpShutdownTimeout/time.Second)),
zap.Int("agent_timeout_seconds", int(agentShutdownTimeout/time.Second)),
zap.Int("tracing_timeout_seconds", int(tracingShutdownTimeout/time.Second)))
shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 10*time.Second)
shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), httpShutdownTimeout)
defer shutdownCancel()
if err := server.Shutdown(shutdownCtx); err != nil {
shutdownErrs = append(shutdownErrs, err)
log.Error("HTTP server shutdown error", zap.Error(err))
}
if err := orchestratorSvc.Stop(); err != nil {
shutdownErrs = append(shutdownErrs, err)
log.Error("Orchestrator stop error", zap.Error(err))
}
stopLifecycleManager(lifecycleMgr, log)
if err := stopLifecycleManager(lifecycleMgr, log); err != nil {
shutdownErrs = append(shutdownErrs, err)
}
runCleanups()
// Flush pending OTel spans before exit
traceCtx, traceCancel := context.WithTimeout(context.Background(), 5*time.Second)
traceCtx, traceCancel := context.WithTimeout(context.Background(), tracingShutdownTimeout)
if err := tracing.Shutdown(traceCtx); err != nil {
shutdownErrs = append(shutdownErrs, err)
log.Error("Tracer shutdown error", zap.Error(err))
}
traceCancel()
log.Info("Kandev stopped")
log.Info("Graceful shutdown complete",
zap.Duration("duration", time.Since(start)),
zap.Int("error_count", len(shutdownErrs)))
_ = log.Sync()
}
// stopLifecycleManager gracefully stops all agents and the lifecycle manager.
func stopLifecycleManager(lifecycleMgr *lifecycle.Manager, log *logger.Logger) {
func stopLifecycleManager(lifecycleMgr *lifecycle.Manager, log *logger.Logger) error {
if lifecycleMgr == nil {
return
return nil
}
log.Info("Stopping agents gracefully...")
stopCtx, stopCancel := context.WithTimeout(context.Background(), 5*time.Second)
var shutdownErrs []error
log.Info("Stopping agents gracefully",
zap.Int("timeout_seconds", int(agentShutdownTimeout/time.Second)))
stopCtx, stopCancel := context.WithTimeout(context.Background(), agentShutdownTimeout)
if err := lifecycleMgr.StopAllAgents(stopCtx); err != nil {
shutdownErrs = append(shutdownErrs, err)
log.Error("Graceful agent stop error", zap.Error(err))
}
stopCancel()
if err := lifecycleMgr.Stop(); err != nil {
shutdownErrs = append(shutdownErrs, err)
log.Error("Lifecycle manager stop error", zap.Error(err))
}
if len(shutdownErrs) == 0 {
log.Info("Agents stopped gracefully")
}
return errors.Join(shutdownErrs...)
}
@@ -12,7 +12,9 @@ import (
"github.com/gin-gonic/gin"
"github.com/jmoiron/sqlx"
"github.com/kandev/kandev/internal/agent/executor"
"github.com/kandev/kandev/internal/agent/runtime/lifecycle"
"github.com/kandev/kandev/internal/agentctl/server/process"
"github.com/kandev/kandev/internal/common/config"
"github.com/kandev/kandev/internal/common/logger"
"github.com/kandev/kandev/internal/db"
@@ -114,6 +116,40 @@ func TestAppendAgentctlStatusMessage_IncludesWorkspacePathForReload(t *testing.T
}
}
func TestStopLifecycleManagerAllowsAgentctlInstanceCleanupWindow(t *testing.T) {
log, err := logger.NewLogger(logger.LoggingConfig{
Level: "error",
Format: "console",
OutputPath: "stdout",
})
if err != nil {
t.Fatalf("NewLogger: %v", err)
}
stopper := &shutdownDeadlineExecutor{}
execRegistry := lifecycle.NewExecutorRegistry(log)
execRegistry.Register(stopper)
mgr := lifecycle.NewManager(nil, nil, execRegistry, nil, nil, nil, lifecycle.ExecutorFallbackDeny, t.TempDir(), log)
if err := mgr.ExecutionStoreForTesting().Add(&lifecycle.AgentExecution{
ID: "exec-1",
TaskID: "task-1",
SessionID: "sess-1",
RuntimeName: executor.NameStandalone,
}); err != nil {
t.Fatalf("add execution: %v", err)
}
startedAt := time.Now()
_ = stopLifecycleManager(mgr, log)
if stopper.deadline.IsZero() {
t.Fatal("StopInstance was not called with a deadline")
}
if got := stopper.deadline.Sub(startedAt); got < 15*time.Second {
t.Fatalf("agent shutdown timeout = %v, want at least 15s for agentctl instance cleanup", got)
}
}
func TestBootInitialStateIncludesFeatureFlags(t *testing.T) {
state := bootInitialState(
context.Background(),
@@ -137,6 +173,44 @@ func TestBootInitialStateIncludesFeatureFlags(t *testing.T) {
}
}
type shutdownDeadlineExecutor struct {
deadline time.Time
}
func (s *shutdownDeadlineExecutor) Name() executor.Name { return executor.NameStandalone }
func (s *shutdownDeadlineExecutor) HealthCheck(context.Context) error { return nil }
func (s *shutdownDeadlineExecutor) CreateInstance(
context.Context,
*lifecycle.ExecutorCreateRequest,
) (*lifecycle.ExecutorInstance, error) {
return nil, nil
}
func (s *shutdownDeadlineExecutor) StopInstance(
ctx context.Context,
_ *lifecycle.ExecutorInstance,
_ bool,
) error {
if deadline, ok := ctx.Deadline(); ok {
s.deadline = deadline
}
return nil
}
func (s *shutdownDeadlineExecutor) RecoverInstances(context.Context) ([]*lifecycle.ExecutorInstance, error) {
return nil, nil
}
func (s *shutdownDeadlineExecutor) GetInteractiveRunner() *process.InteractiveRunner { return nil }
func (s *shutdownDeadlineExecutor) RequiresCloneURL() bool { return false }
func (s *shutdownDeadlineExecutor) ShouldApplyPreferredShell() bool { return true }
func (s *shutdownDeadlineExecutor) IsAlwaysResumable() bool { return false }
func TestBootInitialStateHomeIncludesKanbanFirstPaintState(t *testing.T) {
taskSvc, workflowSvc := newBootStateTestServices(t)
ctx := context.Background()
+6 -1
View File
@@ -1678,6 +1678,9 @@ func awaitShutdown(
// ============================================
quit := make(chan os.Signal, 2)
signal.Notify(quit, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP)
log.Debug("shutdown signal handler armed",
zap.Int("pid", os.Getpid()),
zap.Int("ppid", os.Getppid()))
sig := <-quit
// If we get a second signal, exit immediately.
@@ -1688,6 +1691,8 @@ func awaitShutdown(
os.Exit(1)
}()
log.Info("Received shutdown signal", zap.String("signal", sig.String()))
log.Info("Received shutdown signal",
zap.String("signal", sig.String()),
zap.Int("pid", os.Getpid()))
runGracefulShutdown(server, orchestratorSvc, lifecycleMgr, runCleanups, log)
}
@@ -4,21 +4,21 @@ import "testing"
func TestIsValidBaseBranchRef(t *testing.T) {
cases := map[string]bool{
"main": true,
"release/v2": true,
"feature/foo-bar": true,
"origin/main": true,
"origin/release/v2": true,
"": false,
"main/": false, // trailing slash
"origin/main/": false, // trailing slash after origin strip
"a//b": false, // consecutive slashes
"bad..ref": false, // path traversal
"feature.lock": false, // .lock suffix
"bad branch": false, // space
"bad;rm -rf": false, // shell metacharacters
"-flag": false, // leading dash (regex requires alphanumeric first)
"/leading": false, // leading slash
"main": true,
"release/v2": true,
"feature/foo-bar": true,
"origin/main": true,
"origin/release/v2": true,
"": false,
"main/": false, // trailing slash
"origin/main/": false, // trailing slash after origin strip
"a//b": false, // consecutive slashes
"bad..ref": false, // path traversal
"feature.lock": false, // .lock suffix
"bad branch": false, // space
"bad;rm -rf": false, // shell metacharacters
"-flag": false, // leading dash (regex requires alphanumeric first)
"/leading": false, // leading slash
}
for ref, want := range cases {
if got := IsValidBaseBranchRef(ref); got != want {
+332 -19
View File
@@ -1,22 +1,37 @@
package launcher
import (
"errors"
"fmt"
"io"
"os"
"os/exec"
"os/signal"
"sync"
"sync/atomic"
"syscall"
"time"
)
const capturedOutputLimit = 64 * 1024
// Keep this above the backend's serial graceful-shutdown budgets so the
// launcher does not preempt agent/session cleanup that is still in progress.
const managedProcessShutdownGrace = 75 * time.Second
const managedProcessForceKillWait = 2 * time.Second
var launcherShutdownDebug atomic.Bool
var launcherStatusOutput io.Writer = os.Stderr
var launcherExit = os.Exit
type processSupervisor struct {
mu sync.Mutex
children []*managedProcess
mu sync.Mutex
children []*managedProcess
shutdownOnce sync.Once
}
type managedProcess struct {
label string
cmd *exec.Cmd
exitCode int
exited bool
@@ -24,33 +39,119 @@ type managedProcess struct {
done chan struct{}
}
type managedProcessShutdownResult struct {
label string
pid int
duration time.Duration
graceful bool
forceKilled bool
err error
}
type shutdownSummary struct {
graceful int
forceKilled int
failed int
}
func newSupervisor() *processSupervisor {
return &processSupervisor{}
}
func setLauncherShutdownDebug(enabled bool) {
launcherShutdownDebug.Store(enabled)
}
func shutdownDebugf(format string, args ...interface{}) {
if !launcherShutdownDebug.Load() {
return
}
fmt.Fprintf(os.Stderr, "[kandev] [SHUTDOWN-DEBUG] "+format+"\n", args...)
}
func launcherInfof(format string, args ...interface{}) {
_, _ = fmt.Fprintf(launcherStatusOutput, "[kandev] "+format+"\n", args...)
}
func (s *processSupervisor) add(proc *managedProcess) {
s.mu.Lock()
defer s.mu.Unlock()
s.children = append(s.children, proc)
shutdownDebugf("supervisor add child pid=%d total_children=%d", proc.cmd.Process.Pid, len(s.children))
}
func (s *processSupervisor) shutdown(reason string) {
fmt.Printf("[kandev] shutting down (%s)...\n", reason)
s.shutdownOnce.Do(func() {
s.runShutdown(reason)
})
}
func (s *processSupervisor) runShutdown(reason string) {
s.mu.Lock()
children := append([]*managedProcess(nil), s.children...)
s.mu.Unlock()
for _, child := range children {
child.kill()
start := time.Now()
launcherInfof("graceful shutdown started (reason=%s, timeout=%s, processes=%d)",
reason, managedProcessShutdownGrace, len(children))
shutdownDebugf("launcher shutdown begin reason=%q launcher_pid=%d children=%d", reason, os.Getpid(), len(children))
results := make([]managedProcessShutdownResult, len(children))
var wg sync.WaitGroup
for i, child := range children {
wg.Add(1)
go func(i int, child *managedProcess) {
defer wg.Done()
results[i] = child.kill()
}(i, child)
}
wg.Wait()
logShutdownComplete(time.Since(start), results)
shutdownDebugf("launcher shutdown complete reason=%q", reason)
}
func (s *processSupervisor) forceKillAll(reason string) []managedProcessShutdownResult {
s.mu.Lock()
children := append([]*managedProcess(nil), s.children...)
s.mu.Unlock()
shutdownDebugf("launcher force kill begin reason=%q launcher_pid=%d children=%d", reason, os.Getpid(), len(children))
results := make([]managedProcessShutdownResult, len(children))
var wg sync.WaitGroup
for i, child := range children {
wg.Add(1)
go func(i int, child *managedProcess) {
defer wg.Done()
results[i] = child.forceKill(reason)
}(i, child)
}
wg.Wait()
shutdownDebugf("launcher force kill complete reason=%q", reason)
return results
}
func (s *processSupervisor) attachSignals() {
ch := make(chan os.Signal, 1)
signal.Notify(ch, os.Interrupt, syscall.SIGTERM)
shutdownDebugf("launcher signal handler armed launcher_pid=%d", os.Getpid())
go func() {
sig := <-ch
s.shutdown("signal " + sig.String())
os.Exit(0)
shutdownDebugf("launcher received signal=%s launcher_pid=%d", sig.String(), os.Getpid())
shutdownDone := make(chan struct{})
go func() {
s.shutdown("signal " + sig.String())
close(shutdownDone)
}()
select {
case nextSig := <-ch:
shutdownDebugf("launcher received signal during shutdown=%s launcher_pid=%d", nextSig.String(), os.Getpid())
forceStart := time.Now()
launcherInfof("forced shutdown after second signal (signal=%s)", nextSig.String())
results := s.forceKillAll("second signal " + nextSig.String())
logForcedShutdownComplete(time.Since(forceStart), results)
signal.Stop(ch)
launcherExit(1)
case <-shutdownDone:
signal.Stop(ch)
launcherExit(0)
}
}()
}
@@ -61,18 +162,23 @@ func startProcess(command string, args []string, cwd string, env []string, quiet
cmd.Stdin = nil
configureManagedProcess(cmd)
stdout := newLimitedBuffer(capturedOutputLimit)
if quiet {
cmd.Stdout = stdout
cmd.Stderr = os.Stderr
} else {
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
var stdoutSink io.Writer
if !quiet {
stdoutSink = os.Stdout
}
cmd.Stdout = newProcessOutput(stdout, stdoutSink, os.Stderr, label+".stdout")
cmd.Stderr = newProcessOutput(nil, os.Stderr, nil, label+".stderr")
supervisor.mu.Lock()
if err := cmd.Start(); err != nil {
supervisor.mu.Unlock()
return nil, nil, err
}
proc := &managedProcess{cmd: cmd, done: make(chan struct{})}
supervisor.add(proc)
proc := &managedProcess{label: label, cmd: cmd, done: make(chan struct{})}
supervisor.children = append(supervisor.children, proc)
childCount := len(supervisor.children)
supervisor.mu.Unlock()
shutdownDebugf("managed process started label=%q pid=%d command=%q args=%q cwd=%q", label, cmd.Process.Pid, command, args, cwd)
shutdownDebugf("supervisor add child pid=%d total_children=%d", proc.cmd.Process.Pid, childCount)
go func() {
err := cmd.Wait()
code := 0
@@ -82,6 +188,11 @@ func startProcess(command string, args []string, cwd string, env []string, quiet
code = exitErr.ExitCode()
}
}
state := ""
if cmd.ProcessState != nil {
state = cmd.ProcessState.String()
}
shutdownDebugf("managed process wait complete label=%q pid=%d code=%d state=%q err=%v", label, cmd.Process.Pid, code, state, err)
proc.mu.Lock()
proc.exitCode = code
proc.exited = true
@@ -108,6 +219,47 @@ type limitedBuffer struct {
buf []byte
}
type processOutput struct {
mu sync.Mutex
buffer *limitedBuffer
sink io.Writer
fallbackSink io.Writer
label string
sinkDisabled bool
}
func newProcessOutput(buffer *limitedBuffer, sink io.Writer, fallbackSink io.Writer, label string) *processOutput {
return &processOutput{buffer: buffer, sink: sink, fallbackSink: fallbackSink, label: label}
}
func (w *processOutput) Write(p []byte) (int, error) {
if w.buffer != nil {
_, _ = w.buffer.Write(p)
}
w.mu.Lock()
defer w.mu.Unlock()
if w.sink == nil || w.sinkDisabled {
return len(p), nil
}
if _, err := w.sink.Write(p); err != nil {
if w.fallbackSink != nil && w.fallbackSink != w.sink {
shutdownDebugf("switching broken output sink to fallback label=%q err=%v", w.label, err)
w.sink = w.fallbackSink
w.fallbackSink = nil
var fallbackErr error
if _, fallbackErr = w.sink.Write(p); fallbackErr == nil {
return len(p), nil
}
err = fallbackErr
}
w.sinkDisabled = true
shutdownDebugf("disabling broken output sink label=%q err=%v", w.label, err)
launcherInfof("warning: disabling %s output sink after write failure: %v", w.label, err)
}
return len(p), nil
}
func newLimitedBuffer(limit int) *limitedBuffer {
return &limitedBuffer{limit: limit}
}
@@ -134,14 +286,132 @@ func (p *managedProcess) Exited() (bool, int) {
return p.exited, p.exitCode
}
func (p *managedProcess) kill() {
func (p *managedProcess) kill() managedProcessShutdownResult {
start := time.Now()
result := managedProcessShutdownResult{label: p.label}
if p.cmd.Process == nil {
return
shutdownDebugf("managed process kill skipped: process nil")
result.err = fmt.Errorf("process is nil")
return result
}
if err := killManagedProcessGroup(p.cmd.Process.Pid); err != nil {
pid := p.cmd.Process.Pid
result.pid = pid
select {
case <-p.done:
result.duration = time.Since(start)
result.graceful = true
shutdownDebugf("managed process kill skipped; already exited label=%q pid=%d", p.label, pid)
return result
default:
}
shutdownDebugf("managed process kill begin label=%q pid=%d grace=%s", p.label, pid, managedProcessShutdownGrace)
shutdownDebugf("managed process group SIGTERM requested label=%q pgid=%d", p.label, pid)
if err := terminateManagedProcessGroup(pid); err != nil {
if errors.Is(err, syscall.ESRCH) {
result.duration = time.Since(start)
result.graceful = true
shutdownDebugf("managed process group already gone label=%q pid=%d", p.label, pid)
return result
}
shutdownDebugf("managed process group SIGTERM failed pid=%d err=%v; killing process", pid, err)
shutdownDebugf("managed process SIGKILL requested label=%q pid=%d reason=%q", p.label, pid, "sigterm_failed")
_ = p.cmd.Process.Kill()
result.forceKilled = true
result.err = err
if waitForManagedProcessKillDone(p.done, managedProcessForceKillWait) {
shutdownDebugf("managed process killed after SIGTERM failure pid=%d", pid)
} else {
result.err = errors.Join(result.err, fmt.Errorf("timed out waiting for process %d after SIGKILL", pid))
shutdownDebugf("managed process kill wait timed out after SIGTERM failure pid=%d", pid)
}
result.duration = time.Since(start)
return result
}
shutdownDebugf("managed process group SIGTERM sent pgid=%d", pid)
select {
case <-p.done:
result.duration = time.Since(start)
result.graceful = true
shutdownDebugf("managed process exited within grace pid=%d", pid)
return result
case <-time.After(managedProcessShutdownGrace):
}
shutdownDebugf("managed process grace expired; sending SIGKILL pgid=%d", pid)
result.forceKilled = true
if err := killManagedProcessGroup(pid); err != nil {
if errors.Is(err, syscall.ESRCH) {
result.forceKilled = false
result.graceful = true
result.duration = time.Since(start)
shutdownDebugf("managed process group already gone before SIGKILL label=%q pid=%d", p.label, pid)
return result
}
shutdownDebugf("managed process group SIGKILL failed pid=%d err=%v; killing process", pid, err)
shutdownDebugf("managed process SIGKILL requested label=%q pid=%d reason=%q", p.label, pid, "process_group_kill_failed")
_ = p.cmd.Process.Kill()
result.err = err
}
if waitForManagedProcessKillDone(p.done, managedProcessForceKillWait) {
shutdownDebugf("managed process killed after grace pid=%d", pid)
} else {
result.err = errors.Join(result.err, fmt.Errorf("timed out waiting for process %d after SIGKILL", pid))
shutdownDebugf("managed process kill wait timed out after SIGKILL pid=%d", pid)
}
result.duration = time.Since(start)
return result
}
func (p *managedProcess) forceKill(reason string) managedProcessShutdownResult {
start := time.Now()
result := managedProcessShutdownResult{label: p.label}
if p.cmd.Process == nil {
shutdownDebugf("managed process force kill skipped: process nil")
result.err = fmt.Errorf("process is nil")
return result
}
pid := p.cmd.Process.Pid
result.pid = pid
select {
case <-p.done:
result.duration = time.Since(start)
result.graceful = true
shutdownDebugf("managed process force kill skipped; already exited label=%q pid=%d reason=%q", p.label, pid, reason)
return result
default:
}
shutdownDebugf("managed process group SIGKILL requested label=%q pgid=%d reason=%q", p.label, pid, reason)
result.forceKilled = true
if err := killManagedProcessGroup(pid); err != nil {
if errors.Is(err, syscall.ESRCH) {
result.forceKilled = false
result.graceful = true
result.duration = time.Since(start)
shutdownDebugf("managed process group already gone before force kill label=%q pid=%d", p.label, pid)
return result
}
shutdownDebugf("managed process group SIGKILL failed pid=%d err=%v; killing process", pid, err)
shutdownDebugf("managed process SIGKILL requested label=%q pid=%d reason=%q", p.label, pid, "force_kill_group_failed")
_ = p.cmd.Process.Kill()
result.err = err
}
if waitForManagedProcessKillDone(p.done, managedProcessForceKillWait) {
result.duration = time.Since(start)
shutdownDebugf("managed process force killed pid=%d", pid)
return result
}
result.duration = time.Since(start)
result.err = errors.Join(result.err, fmt.Errorf("timed out waiting for process %d after SIGKILL", pid))
shutdownDebugf("managed process force kill wait timed out pid=%d", pid)
return result
}
func waitForManagedProcessKillDone(done <-chan struct{}, timeout time.Duration) bool {
select {
case <-done:
return true
case <-time.After(timeout):
return false
}
<-p.done
}
func waitForAppExit(supervisor *processSupervisor, backend *restartableBackend) int {
@@ -149,3 +419,46 @@ func waitForAppExit(supervisor *processSupervisor, backend *restartableBackend)
supervisor.shutdown("backend exit")
return code
}
func logForcedShutdownComplete(duration time.Duration, results []managedProcessShutdownResult) {
summary := summarizeShutdown(results)
launcherInfof("forced shutdown complete (duration=%s, graceful=%d, force_killed=%d, failed=%d)",
duration.Round(time.Millisecond), summary.graceful, summary.forceKilled, summary.failed)
}
func logShutdownComplete(duration time.Duration, results []managedProcessShutdownResult) {
summary := summarizeShutdown(results)
launcherInfof("graceful shutdown complete (duration=%s, graceful=%d, force_killed=%d, failed=%d)",
duration.Round(time.Millisecond), summary.graceful, summary.forceKilled, summary.failed)
for _, result := range results {
if result.forceKilled || result.err != nil {
label := result.label
if label == "" {
label = "process"
}
if result.err != nil {
launcherInfof("shutdown detail: %s pid=%d required force cleanup after %s: %v",
label, result.pid, result.duration.Round(time.Millisecond), result.err)
continue
}
launcherInfof("shutdown detail: %s pid=%d required SIGKILL after %s",
label, result.pid, result.duration.Round(time.Millisecond))
}
}
}
func summarizeShutdown(results []managedProcessShutdownResult) shutdownSummary {
var summary shutdownSummary
for _, result := range results {
if result.forceKilled {
summary.forceKilled++
}
if result.err != nil {
summary.failed++
}
if result.graceful {
summary.graceful++
}
}
return summary
}
@@ -1,4 +1,4 @@
//go:build !linux && !darwin
//go:build !linux && !darwin && !windows
package launcher
@@ -7,9 +7,16 @@ import (
"os/exec"
)
func ignoreBrokenPipeSignal() {
}
func configureManagedProcess(_ *exec.Cmd) {
}
func killManagedProcessGroup(_ int) error {
return errors.New("process groups are not supported on this platform")
}
func terminateManagedProcessGroup(_ int) error {
return errors.New("process groups are not supported on this platform")
}
@@ -4,9 +4,14 @@ package launcher
import (
"os/exec"
"os/signal"
"syscall"
)
func ignoreBrokenPipeSignal() {
signal.Ignore(syscall.SIGPIPE)
}
func configureManagedProcess(cmd *exec.Cmd) {
cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
}
@@ -14,3 +19,7 @@ func configureManagedProcess(cmd *exec.Cmd) {
func killManagedProcessGroup(pid int) error {
return syscall.Kill(-pid, syscall.SIGKILL)
}
func terminateManagedProcessGroup(pid int) error {
return syscall.Kill(-pid, syscall.SIGTERM)
}
@@ -3,10 +3,20 @@
package launcher
import (
"bytes"
"os"
"os/exec"
"os/signal"
"path/filepath"
"strings"
"sync"
"syscall"
"testing"
"time"
)
const launcherSignalHelperEnv = "KANDEV_LAUNCHER_SIGNAL_HELPER"
func TestConfigureManagedProcessCreatesProcessGroup(t *testing.T) {
cmd := exec.Command("kandev")
configureManagedProcess(cmd)
@@ -15,3 +25,253 @@ func TestConfigureManagedProcessCreatesProcessGroup(t *testing.T) {
t.Fatalf("configureManagedProcess should set Setpgid")
}
}
func TestManagedProcessKillSendsGracefulSignalBeforeForceKill(t *testing.T) {
tempDir := t.TempDir()
readyFile := filepath.Join(tempDir, "ready")
termFile := filepath.Join(tempDir, "term")
cmd := exec.Command(os.Args[0], "-test.run=TestLauncherSignalHelper")
cmd.Env = append(os.Environ(),
launcherSignalHelperEnv+"=1",
"KANDEV_LAUNCHER_SIGNAL_HELPER_READY_FILE="+readyFile,
"KANDEV_LAUNCHER_SIGNAL_HELPER_FILE="+termFile,
)
configureManagedProcess(cmd)
if err := cmd.Start(); err != nil {
t.Fatalf("start fixture: %v", err)
}
proc := &managedProcess{cmd: cmd, done: make(chan struct{})}
go func() {
err := cmd.Wait()
code := 0
if err != nil {
code = 1
if exitErr, ok := err.(*exec.ExitError); ok {
code = exitErr.ExitCode()
}
}
proc.mu.Lock()
proc.exitCode = code
proc.exited = true
proc.mu.Unlock()
close(proc.done)
}()
t.Cleanup(func() {
if exited, _ := proc.Exited(); !exited {
_ = killManagedProcessGroup(cmd.Process.Pid)
<-proc.done
}
})
waitForFile(t, readyFile)
proc.kill()
raw, err := os.ReadFile(termFile)
if err != nil {
t.Fatalf("expected fixture to handle SIGTERM before force kill: %v", err)
}
if string(raw) != "term" {
t.Fatalf("SIGTERM marker = %q, want term", string(raw))
}
}
func TestAttachSignalsSingleSignalGracefulShutdown(t *testing.T) {
tempDir := t.TempDir()
termFile := filepath.Join(tempDir, "term")
proc := startManagedSignalHelper(t, termFile, "")
exitCh, output := captureLauncherExit(t)
supervisor := newSupervisor()
supervisor.add(proc)
supervisor.attachSignals()
sendLauncherTestSignal(t, os.Interrupt)
waitForLauncherExitCode(t, exitCh, 0)
waitForManagedProcessDone(t, proc, 5*time.Second)
if raw, err := os.ReadFile(termFile); err != nil || string(raw) != "term" {
t.Fatalf("SIGTERM marker = %q, %v; want term, nil", string(raw), err)
}
if got := output.String(); !strings.Contains(got, "graceful shutdown complete") {
t.Fatalf("shutdown output missing graceful completion: %q", got)
}
}
func TestAttachSignalsSecondSignalForceKillsChildren(t *testing.T) {
tempDir := t.TempDir()
termFile := filepath.Join(tempDir, "term")
proc := startManagedSignalHelper(t, termFile, "hold-after-term")
exitCh, output := captureLauncherExit(t)
supervisor := newSupervisor()
supervisor.add(proc)
supervisor.attachSignals()
sendLauncherTestSignal(t, os.Interrupt)
waitForFile(t, termFile)
sendLauncherTestSignal(t, os.Interrupt)
waitForLauncherExitCode(t, exitCh, 1)
waitForManagedProcessDone(t, proc, 5*time.Second)
waitForOutputContains(t, output, "forced shutdown after second signal")
waitForOutputContains(t, output, "forced shutdown complete")
waitForOutputContains(t, output, "graceful shutdown complete")
}
func TestLauncherSignalHelper(t *testing.T) {
if os.Getenv(launcherSignalHelperEnv) != "1" {
return
}
termFile := os.Getenv("KANDEV_LAUNCHER_SIGNAL_HELPER_FILE")
if termFile == "" {
os.Exit(2)
}
signals := make(chan os.Signal, 1)
signal.Notify(signals, syscall.SIGTERM)
readyFile := os.Getenv("KANDEV_LAUNCHER_SIGNAL_HELPER_READY_FILE")
if readyFile == "" {
os.Exit(4)
}
if err := os.WriteFile(readyFile, []byte("ready"), 0o600); err != nil {
os.Exit(5)
}
<-signals
if err := os.WriteFile(termFile, []byte("term"), 0o600); err != nil {
os.Exit(3)
}
if os.Getenv("KANDEV_LAUNCHER_SIGNAL_HELPER_MODE") == "hold-after-term" {
for {
time.Sleep(time.Hour)
}
}
os.Exit(0)
}
func startManagedSignalHelper(t *testing.T, termFile string, mode string) *managedProcess {
t.Helper()
readyFile := filepath.Join(t.TempDir(), "ready")
cmd := exec.Command(os.Args[0], "-test.run=TestLauncherSignalHelper")
cmd.Env = append(os.Environ(),
launcherSignalHelperEnv+"=1",
"KANDEV_LAUNCHER_SIGNAL_HELPER_READY_FILE="+readyFile,
"KANDEV_LAUNCHER_SIGNAL_HELPER_FILE="+termFile,
"KANDEV_LAUNCHER_SIGNAL_HELPER_MODE="+mode,
)
configureManagedProcess(cmd)
if err := cmd.Start(); err != nil {
t.Fatalf("start fixture: %v", err)
}
proc := &managedProcess{label: "fixture", cmd: cmd, done: make(chan struct{})}
go func() {
err := cmd.Wait()
code := 0
if err != nil {
code = 1
if exitErr, ok := err.(*exec.ExitError); ok {
code = exitErr.ExitCode()
}
}
proc.mu.Lock()
proc.exitCode = code
proc.exited = true
proc.mu.Unlock()
close(proc.done)
}()
t.Cleanup(func() {
if exited, _ := proc.Exited(); !exited {
_ = killManagedProcessGroup(cmd.Process.Pid)
waitForManagedProcessDone(t, proc, 5*time.Second)
}
})
waitForFile(t, readyFile)
return proc
}
func captureLauncherExit(t *testing.T) (chan int, *safeOutput) {
t.Helper()
exitCh := make(chan int, 1)
output := &safeOutput{}
oldExit := launcherExit
oldStatusOutput := launcherStatusOutput
launcherExit = func(code int) {
exitCh <- code
}
launcherStatusOutput = output
t.Cleanup(func() {
launcherExit = oldExit
launcherStatusOutput = oldStatusOutput
})
return exitCh, output
}
func sendLauncherTestSignal(t *testing.T, sig os.Signal) {
t.Helper()
proc, err := os.FindProcess(os.Getpid())
if err != nil {
t.Fatalf("find current process: %v", err)
}
if err := proc.Signal(sig); err != nil {
t.Fatalf("send signal %v: %v", sig, err)
}
}
func waitForLauncherExitCode(t *testing.T, exitCh <-chan int, want int) {
t.Helper()
select {
case got := <-exitCh:
if got != want {
t.Fatalf("launcher exit code = %d, want %d", got, want)
}
case <-time.After(5 * time.Second):
t.Fatalf("timed out waiting for launcher exit code %d", want)
}
}
func waitForManagedProcessDone(t *testing.T, proc *managedProcess, timeout time.Duration) {
t.Helper()
select {
case <-proc.done:
case <-time.After(timeout):
t.Fatalf("timed out waiting for managed process pid=%d", proc.cmd.Process.Pid)
}
}
func waitForOutputContains(t *testing.T, output *safeOutput, want string) {
t.Helper()
for range 500 {
if strings.Contains(output.String(), want) {
return
}
time.Sleep(10 * time.Millisecond)
}
t.Fatalf("shutdown output missing %q: %q", want, output.String())
}
type safeOutput struct {
mu sync.Mutex
buf bytes.Buffer
}
func (o *safeOutput) Write(p []byte) (int, error) {
o.mu.Lock()
defer o.mu.Unlock()
return o.buf.Write(p)
}
func (o *safeOutput) String() string {
o.mu.Lock()
defer o.mu.Unlock()
return o.buf.String()
}
func waitForFile(t *testing.T, path string) {
t.Helper()
for range 100 {
if _, err := os.Stat(path); err == nil {
return
}
time.Sleep(10 * time.Millisecond)
}
t.Fatalf("timed out waiting for %s", path)
}
@@ -0,0 +1,32 @@
//go:build windows
package launcher
import (
"fmt"
"os/exec"
"syscall"
"github.com/kandev/kandev/internal/agentctl/server/winproc"
)
func ignoreBrokenPipeSignal() {
}
func configureManagedProcess(cmd *exec.Cmd) {
cmd.SysProcAttr = &syscall.SysProcAttr{
CreationFlags: syscall.CREATE_NEW_PROCESS_GROUP,
}
}
func killManagedProcessGroup(pid int) error {
return runLauncherTaskkill("/F", "/T", "/PID", fmt.Sprintf("%d", pid))
}
func terminateManagedProcessGroup(pid int) error {
return runLauncherTaskkill("/T", "/PID", fmt.Sprintf("%d", pid))
}
func runLauncherTaskkill(args ...string) error {
return winproc.RunTaskkill(args...)
}
@@ -0,0 +1,47 @@
//go:build windows
package launcher
import (
"os"
"os/exec"
"testing"
)
const launcherWindowsHelperEnv = "KANDEV_LAUNCHER_WINDOWS_HELPER"
func TestWindowsManagedProcessForceKillTreatsAlreadyExitedPIDAsGraceful(t *testing.T) {
cmd := exec.Command(os.Args[0], "-test.run=TestWindowsLauncherHelper")
cmd.Env = append(os.Environ(), launcherWindowsHelperEnv+"=1")
configureManagedProcess(cmd)
if err := cmd.Start(); err != nil {
t.Fatalf("start helper: %v", err)
}
pid := cmd.Process.Pid
if err := cmd.Wait(); err != nil {
t.Fatalf("wait helper: %v", err)
}
proc := &managedProcess{
label: "already-exited",
cmd: cmd,
done: make(chan struct{}),
}
result := proc.forceKill("test")
if result.err != nil {
t.Fatalf("forceKill err = %v, want nil", result.err)
}
if !result.graceful || result.forceKilled {
t.Fatalf("forceKill result graceful=%v forceKilled=%v, want true/false", result.graceful, result.forceKilled)
}
if result.pid != pid {
t.Fatalf("forceKill pid = %d, want %d", result.pid, pid)
}
}
func TestWindowsLauncherHelper(t *testing.T) {
if os.Getenv(launcherWindowsHelperEnv) != "1" {
return
}
os.Exit(0)
}
@@ -2,7 +2,10 @@ package launcher
import (
"bytes"
"errors"
"strings"
"testing"
"time"
)
func TestLimitedBufferKeepsOnlyTail(t *testing.T) {
@@ -31,3 +34,126 @@ func TestLimitedBufferBytesReturnsSnapshot(t *testing.T) {
t.Fatalf("mutating snapshot changed buffer: %q", buf.Bytes())
}
}
func TestProcessOutputKeepsDrainingAfterSinkBreaks(t *testing.T) {
var output bytes.Buffer
oldStatusOutput := launcherStatusOutput
launcherStatusOutput = &output
t.Cleanup(func() {
launcherStatusOutput = oldStatusOutput
})
sink := &failingWriter{err: errors.New("broken pipe")}
buf := newLimitedBuffer(20)
out := newProcessOutput(buf, sink, nil, "test")
if n, err := out.Write([]byte("first")); err != nil || n != len("first") {
t.Fatalf("first Write() = (%d, %v), want (%d, nil)", n, err, len("first"))
}
if n, err := out.Write([]byte("second")); err != nil || n != len("second") {
t.Fatalf("second Write() = (%d, %v), want (%d, nil)", n, err, len("second"))
}
if sink.calls != 1 {
t.Fatalf("sink calls = %d, want 1", sink.calls)
}
if got := string(buf.Bytes()); got != "firstsecond" {
t.Fatalf("buffer = %q, want firstsecond", got)
}
if got := output.String(); !strings.Contains(got, "warning: disabling test output sink") {
t.Fatalf("status output did not warn about disabled sink: %q", got)
}
}
func TestProcessOutputFallsBackAfterSinkBreaks(t *testing.T) {
sink := &failingWriter{err: errors.New("broken pipe")}
var fallback bytes.Buffer
out := newProcessOutput(nil, sink, &fallback, "test")
if n, err := out.Write([]byte("first")); err != nil || n != len("first") {
t.Fatalf("first Write() = (%d, %v), want (%d, nil)", n, err, len("first"))
}
if n, err := out.Write([]byte("second")); err != nil || n != len("second") {
t.Fatalf("second Write() = (%d, %v), want (%d, nil)", n, err, len("second"))
}
if sink.calls != 1 {
t.Fatalf("sink calls = %d, want 1", sink.calls)
}
if got := fallback.String(); got != "firstsecond" {
t.Fatalf("fallback output = %q, want firstsecond", got)
}
}
func TestSummarizeShutdownCountsGracefulForceKilledAndFailed(t *testing.T) {
errStop := errors.New("stop failed")
got := summarizeShutdown([]managedProcessShutdownResult{
{graceful: true},
{forceKilled: true},
{forceKilled: true, err: errStop},
})
if got.graceful != 1 {
t.Fatalf("graceful = %d, want 1", got.graceful)
}
if got.forceKilled != 2 {
t.Fatalf("forceKilled = %d, want 2", got.forceKilled)
}
if got.failed != 1 {
t.Fatalf("failed = %d, want 1", got.failed)
}
}
func TestSupervisorShutdownRunsOnce(t *testing.T) {
var output bytes.Buffer
oldStatusOutput := launcherStatusOutput
launcherStatusOutput = &output
t.Cleanup(func() {
launcherStatusOutput = oldStatusOutput
})
supervisor := newSupervisor()
supervisor.shutdown("signal interrupt")
supervisor.shutdown("backend exit")
got := output.String()
if count := strings.Count(got, "graceful shutdown started"); count != 1 {
t.Fatalf("shutdown start log count = %d, want 1; output:\n%s", count, got)
}
if count := strings.Count(got, "graceful shutdown complete"); count != 1 {
t.Fatalf("shutdown complete log count = %d, want 1; output:\n%s", count, got)
}
if strings.Contains(got, "backend exit") {
t.Fatalf("second shutdown reason was logged; output:\n%s", got)
}
}
func TestWaitForManagedProcessKillDoneReturnsOnClose(t *testing.T) {
done := make(chan struct{})
close(done)
if !waitForManagedProcessKillDone(done, time.Second) {
t.Fatal("expected closed done channel to return true")
}
}
func TestWaitForManagedProcessKillDoneTimesOut(t *testing.T) {
done := make(chan struct{})
start := time.Now()
if waitForManagedProcessKillDone(done, 10*time.Millisecond) {
t.Fatal("expected open done channel to time out")
}
if elapsed := time.Since(start); elapsed > time.Second {
t.Fatalf("timeout wait took too long: %s", elapsed)
}
}
type failingWriter struct {
calls int
err error
}
func (w *failingWriter) Write(p []byte) (int, error) {
w.calls++
return 0, w.err
}
+17 -6
View File
@@ -9,8 +9,14 @@ import (
)
var (
executablePath = os.Executable
launchManaged = runManagedApp
executablePath = os.Executable
launchManaged = runManagedApp
newSupervisorFn = newSupervisor
launchBackendFn = launchRestartableBackend
waitForHealthFn = waitForHealth
attachSignalsFn = func(supervisor *processSupervisor) {
supervisor.attachSignals()
}
)
func runStart(opts Options) int {
@@ -72,18 +78,23 @@ func resolveLogLevel(opts Options) string {
}
func runManagedApp(cfg managedAppConfig) int {
ignoreBrokenPipeSignal()
logStartup(cfg.Header, cfg.Ports, resolveDatabasePath(), cfg.LogLevel)
setLauncherShutdownDebug(cfg.Opts.Debug || os.Getenv("KANDEV_SHUTDOWN_DEBUG") == "1")
shutdownDebugf("runManagedApp start mode=%q backend=%q backend_cwd=%q debug=%t", cfg.Mode, cfg.Backend, cfg.BackendCWD, cfg.Opts.Debug)
supervisor := newSupervisor()
supervisor.attachSignals()
supervisor := newSupervisorFn()
attachSignalsFn(supervisor)
shutdownDebugf("runManagedApp signal handler attached")
showOutput := cfg.Opts.Verbose || cfg.Opts.Debug
backend, dumpLogs, err := launchRestartableBackend(cfg.Backend, []string{"__backend"}, cfg.BackendCWD, backendEnv(cfg.Ports, cfg.LogLevel, cfg.Opts.Debug), !showOutput, cfg.Ports, cfg.Mode, supervisor)
backend, dumpLogs, err := launchBackendFn(cfg.Backend, []string{"__backend"}, cfg.BackendCWD, backendEnv(cfg.Ports, cfg.LogLevel, cfg.Opts.Debug), !showOutput, cfg.Ports, cfg.Mode, supervisor)
if err != nil {
fmt.Fprintln(os.Stderr, "[kandev] "+err.Error())
return 1
}
shutdownDebugf("runManagedApp backend launched")
fmt.Println("[kandev] starting backend...")
if err := waitForHealth(cfg.Ports.BackendURL, backend, healthTimeout(healthTimeoutReleaseMS), dumpLogs); err != nil {
if err := waitForHealthFn(cfg.Ports.BackendURL, backend, healthTimeout(healthTimeoutReleaseMS), dumpLogs); err != nil {
supervisor.shutdown("backend health failure")
fmt.Fprintln(os.Stderr, "[kandev] "+err.Error())
return 1
@@ -2,7 +2,9 @@ package launcher
import (
"path/filepath"
"reflect"
"testing"
"time"
)
func TestRunStartUsesSelfExecutableAndBackendCWD(t *testing.T) {
@@ -45,3 +47,64 @@ func TestRunStartUsesSelfExecutableAndBackendCWD(t *testing.T) {
t.Fatal("expected Headless option to be preserved")
}
}
func TestRunManagedAppAttachesSignalsBeforeBackendLaunch(t *testing.T) {
oldNewSupervisor := newSupervisorFn
oldLaunchBackend := launchBackendFn
oldAttachSignals := attachSignalsFn
oldWaitForHealth := waitForHealthFn
t.Cleanup(func() {
newSupervisorFn = oldNewSupervisor
launchBackendFn = oldLaunchBackend
attachSignalsFn = oldAttachSignals
waitForHealthFn = oldWaitForHealth
})
var events []string
newSupervisorFn = func() *processSupervisor {
events = append(events, "new-supervisor")
return newSupervisor()
}
launchBackendFn = func(
_ string,
_ []string,
_ string,
_ []string,
_ bool,
_ portConfig,
_ string,
_ *processSupervisor,
) (*restartableBackend, func(), error) {
events = append(events, "launch-backend")
exitCh := make(chan int, 1)
exitCh <- 0
return &restartableBackend{exitCh: exitCh}, func() {}, nil
}
attachSignalsFn = func(_ *processSupervisor) {
events = append(events, "attach-signals")
}
waitForHealthFn = func(_ string, _ childState, _ time.Duration, _ func()) error {
events = append(events, "wait-health")
return nil
}
t.Setenv("KANDEV_HOME_DIR", t.TempDir())
code := runManagedApp(managedAppConfig{
Header: "test",
Mode: "start",
Backend: "kandev",
BackendCWD: t.TempDir(),
Ports: portConfig{
BackendPort: 48123,
BackendURL: "http://localhost:48123",
},
Opts: Options{Headless: true},
})
if code != 0 {
t.Fatalf("runManagedApp() = %d, want 0", code)
}
want := []string{"new-supervisor", "attach-signals", "launch-backend", "wait-health"}
if !reflect.DeepEqual(events, want) {
t.Fatalf("events = %v, want %v", events, want)
}
}
+26
View File
@@ -29,8 +29,17 @@ worktrees, or executor rows behind and the machine slowly runs out of memory.
preserved for retry/diagnosis.
- Agent subprocess shutdown kills the whole agent process group when graceful
shutdown does not finish within the configured stop timeout.
- Agent subprocess shutdown does not treat the command leader exiting as
sufficient while descendants remain alive in the same process group.
- Agentctl instance shutdown never leaves an agent subprocess group alive as a
child of `init`/PID 1.
- Standalone agentctl is isolated from terminal foreground interrupts so the
backend remains the owner of Ctrl+C shutdown sequencing.
- Top-level launchers, including `make start-debug` through `kandev start`, send
the backend a graceful termination signal before any force kill so backend
shutdown can stop agents and agentctl instances.
- Backend shutdown waits long enough for standalone agentctl's instance cleanup
window before reporting shutdown complete.
- Backend startup reconciles stale runtime rows for archived/deleted/missing task
state and attempts safe cleanup instead of treating the rows as live sessions.
- Cleanup is idempotent: repeating archive/delete cleanup, startup reconciliation,
@@ -126,6 +135,12 @@ Allowed transitions:
the resource-specific retry path.
- If an agentctl process exits unexpectedly, its owned agent subprocess group is
killed before agentctl shutdown completes.
- If the user sends Ctrl+C to a standalone Kandev process tree, agentctl does
not receive the terminal interrupt directly; it is stopped through backend
lifecycle shutdown, parent liveness, or an explicit backend signal.
- If the user sends Ctrl+C while running through `make start-debug`, the launcher
forwards a graceful stop to the backend and waits before escalating, rather
than immediately killing the backend process group.
- If startup reconciliation finds rows for archived tasks, deleted tasks, missing
sessions, or terminal sessions with no live runtime, it removes only rows that
are positively confirmed safe to remove.
@@ -162,6 +177,17 @@ Allowed transitions:
- **GIVEN** agentctl is stopped while an ACP child process ignores stdin EOF,
**WHEN** the stop timeout expires, **THEN** the ACP process group is killed and
no ACP child is reparented to PID 1.
- **GIVEN** an ACP wrapper process exits after spawning a native child in the
same process group, **WHEN** agentctl completes shutdown, **THEN** the
remaining process-group descendants are terminated before shutdown is reported
complete.
- **GIVEN** standalone Kandev owns an active agentctl instance, **WHEN** the user
stops Kandev with Ctrl+C, **THEN** backend shutdown supervises agentctl
cleanup and waits for the instance stop window instead of letting agentctl exit
directly from the terminal interrupt.
- **GIVEN** Kandev is running under `make start-debug`, **WHEN** the user stops it
with Ctrl+C, **THEN** the launcher gives the backend a graceful shutdown window
before any force kill so active ACP process groups are reaped.
- **GIVEN** the `executors_running` query fails during archive cleanup, **WHEN**
cleanup evaluates destructive actions, **THEN** it logs the failure and does not
delete runtime tracking rows based on incomplete information.