From ff4e0779d2f04b21c2a2c44c58de9af3d5bf3b3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Sat, 31 Jan 2026 18:06:22 +0000 Subject: [PATCH] feat: improve make dev shutdown (#133) --- apps/backend/Makefile | 2 +- apps/backend/cmd/kandev/main.go | 15 +++- .../internal/agent/lifecycle/manager.go | 29 +++++-- .../agentctl/client/launcher/launcher.go | 2 +- .../agentctl/client/launcher/provider.go | 2 +- apps/cli/src/process.ts | 78 +++++++++++++++++-- 6 files changed, 110 insertions(+), 18 deletions(-) diff --git a/apps/backend/Makefile b/apps/backend/Makefile index 0f4fdf81f4..4ae1259ec6 100644 --- a/apps/backend/Makefile +++ b/apps/backend/Makefile @@ -50,7 +50,7 @@ run: build ## Run in development mode dev: build @echo "Running $(BINARY_NAME) in development mode..." - ./$(BUILD_DIR)/$(BINARY_NAME) + @exec ./$(BUILD_DIR)/$(BINARY_NAME) ## Run tests test: diff --git a/apps/backend/cmd/kandev/main.go b/apps/backend/cmd/kandev/main.go index 6c2c917b4d..239eb1165c 100644 --- a/apps/backend/cmd/kandev/main.go +++ b/apps/backend/cmd/kandev/main.go @@ -121,7 +121,12 @@ func main() { os.Exit(1) } cleanups := make([]func() error, 0) - defer func() { + cleanupsRan := false + runCleanups := func() { + if cleanupsRan { + return + } + cleanupsRan = true for i := len(cleanups) - 1; i >= 0; i-- { if cleanups[i] == nil { continue @@ -130,6 +135,9 @@ func main() { log.Warn("cleanup failed", zap.Error(err)) } } + } + defer func() { + runCleanups() _ = log.Sync() }() logger.SetDefault(log) @@ -621,6 +629,7 @@ func main() { go func() { second := <-quit log.Warn("Received second shutdown signal, forcing exit", zap.String("signal", second.String())) + _ = log.Sync() os.Exit(1) }() @@ -653,5 +662,9 @@ func main() { } } + // Run all cleanups before exiting so logs appear before the shell prompt + runCleanups() + log.Info("Kandev stopped") + _ = log.Sync() } diff --git a/apps/backend/internal/agent/lifecycle/manager.go b/apps/backend/internal/agent/lifecycle/manager.go index a9c88b7735..e3b08a5bbb 100644 --- a/apps/backend/internal/agent/lifecycle/manager.go +++ b/apps/backend/internal/agent/lifecycle/manager.go @@ -601,23 +601,36 @@ func (m *Manager) Stop() error { return nil } -// StopAllAgents attempts a graceful shutdown of all active agents. +// StopAllAgents attempts a graceful shutdown of all active agents concurrently. func (m *Manager) StopAllAgents(ctx context.Context) error { executions := m.executionStore.List() if len(executions) == 0 { return nil } - var errs []error + var wg sync.WaitGroup + errCh := make(chan error, len(executions)) + for _, exec := range executions { - if err := m.StopAgent(ctx, exec.ID, false); err != nil { - errs = append(errs, err) - m.logger.Warn("failed to stop agent during shutdown", - zap.String("execution_id", exec.ID), - zap.Error(err)) - } + wg.Add(1) + go func(e *AgentExecution) { + defer wg.Done() + if err := m.StopAgent(ctx, e.ID, false); err != nil { + errCh <- err + m.logger.Warn("failed to stop agent during shutdown", + zap.String("execution_id", e.ID), + zap.Error(err)) + } + }(exec) } + wg.Wait() + close(errCh) + + var errs []error + for err := range errCh { + errs = append(errs, err) + } return errors.Join(errs...) } diff --git a/apps/backend/internal/agentctl/client/launcher/launcher.go b/apps/backend/internal/agentctl/client/launcher/launcher.go index ff0814bc26..b1ec75d16b 100644 --- a/apps/backend/internal/agentctl/client/launcher/launcher.go +++ b/apps/backend/internal/agentctl/client/launcher/launcher.go @@ -210,7 +210,7 @@ func (l *Launcher) Stop(ctx context.Context) error { select { case <-l.exited: return nil - case <-time.After(2 * time.Second): + case <-time.After(1 * time.Second): return fmt.Errorf("agentctl did not exit after SIGKILL") } } diff --git a/apps/backend/internal/agentctl/client/launcher/provider.go b/apps/backend/internal/agentctl/client/launcher/provider.go index 22f9180241..c22ced3133 100644 --- a/apps/backend/internal/agentctl/client/launcher/provider.go +++ b/apps/backend/internal/agentctl/client/launcher/provider.go @@ -19,7 +19,7 @@ func Provide(ctx context.Context, cfg Config, log *logger.Logger) (*Launcher, fu cleanup := func() error { var stopErr error stopOnce.Do(func() { - stopCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + stopCtx, cancel := context.WithTimeout(context.Background(), 3*time.Second) defer cancel() stopErr = launcher.Stop(stopCtx) }) diff --git a/apps/cli/src/process.ts b/apps/cli/src/process.ts index f0d3ba4eaa..aba3256751 100644 --- a/apps/cli/src/process.ts +++ b/apps/cli/src/process.ts @@ -1,24 +1,34 @@ +import type { ChildProcess } from "node:child_process"; import kill from "tree-kill"; export type ChildLike = { pid?: number | undefined }; +const SHUTDOWN_TIMEOUT_MS = 10000; + export function createProcessSupervisor(): { children: ChildLike[]; shutdown: (reason: string) => Promise; attachSignalHandlers: () => void; } { - let shuttingDown = false; + let shutdownPromise: Promise | null = null; const children: ChildLike[] = []; const shutdown = async (reason: string) => { - if (shuttingDown) return; - shuttingDown = true; + // If already shutting down, wait for the existing shutdown to complete + if (shutdownPromise) { + return shutdownPromise; + } + console.log(`[kandev] shutting down (${reason})...`); - await Promise.all( + + // Wait for all child processes to actually exit, not just for signal to be sent + shutdownPromise = Promise.all( children .filter((child) => child.pid) - .map((child) => new Promise((resolve) => kill(child.pid as number, "SIGTERM", () => resolve()))), - ); + .map((child) => waitForProcessExit(child, SHUTDOWN_TIMEOUT_MS)), + ).then(() => {}); + + return shutdownPromise; }; const onSignal = (signal: NodeJS.Signals) => { @@ -32,3 +42,59 @@ export function createProcessSupervisor(): { return { children, shutdown, attachSignalHandlers }; } + +/** + * Send SIGTERM to a process and wait for it to exit. + * Falls back to SIGKILL if the process doesn't exit within the timeout. + */ +function waitForProcessExit(child: ChildLike, timeoutMs: number): Promise { + return new Promise((resolve) => { + const pid = child.pid as number; + + // Check if this is a ChildProcess with exit event support + const proc = child as ChildProcess; + const hasExitEvent = typeof proc.on === "function" && typeof proc.exitCode !== "undefined"; + + // If process already exited, resolve immediately + if (hasExitEvent && proc.exitCode !== null) { + resolve(); + return; + } + + let resolved = false; + const done = () => { + if (resolved) return; + resolved = true; + resolve(); + }; + + // Set up timeout for fallback SIGKILL + const timeout = setTimeout(() => { + console.log(`[kandev] process ${pid} did not exit in time, sending SIGKILL`); + kill(pid, "SIGKILL", done); + }, timeoutMs); + + // Listen for exit event if available + if (hasExitEvent) { + proc.once("exit", () => { + clearTimeout(timeout); + done(); + }); + } + + // Send SIGTERM + kill(pid, "SIGTERM", (err) => { + // If kill fails (process already gone), we're done + if (err) { + clearTimeout(timeout); + done(); + } + // If no exit event support, resolve after a brief delay + // (tree-kill callback fires when signal sent, not when process exits) + if (!hasExitEvent) { + // For non-ChildProcess objects, we can't wait for exit event + // Just wait for the timeout + } + }); + }); +}