fix: improve startup and lifecycle diagnostics (#3058)

This commit is contained in:
Carlos Florêncio
2026-08-26 13:12:59 +00:00
committed by GitHub
parent 7f24cce795
commit 43a1ce71e4
26 changed files with 1266 additions and 34 deletions
@@ -0,0 +1,28 @@
package api
import (
"net/http"
"net/http/httptest"
"testing"
"github.com/kandev/kandev/internal/agentctl/server/config"
"github.com/kandev/kandev/internal/agentctl/server/instance"
"github.com/kandev/kandev/internal/common/logger"
"github.com/kandev/kandev/pkg/agent"
)
func TestDeleteInstanceUnknownReturnsNotFound(t *testing.T) {
mgr := instance.NewManager(&config.Config{
Defaults: config.InstanceDefaults{Protocol: agent.ProtocolACP},
}, logger.Default())
t.Cleanup(func() { _ = mgr.Shutdown(t.Context()) })
server := NewControlServer(&config.Config{}, mgr, logger.Default())
req := httptest.NewRequest(http.MethodDelete, "/api/v1/instances/missing", nil)
resp := httptest.NewRecorder()
server.Router().ServeHTTP(resp, req)
if resp.Code != http.StatusNotFound {
t.Fatalf("DELETE unknown instance status = %d, want %d", resp.Code, http.StatusNotFound)
}
}
@@ -473,13 +473,27 @@ func (m *Manager) StopInstance(ctx context.Context, id string) error {
if !ok {
return fmt.Errorf("instance %s not found", id)
}
return m.stopInstance(ctx, id, inst)
}
func (m *Manager) stopInstance(ctx context.Context, id string, inst *Instance) error {
inst.stopMu.Lock()
defer inst.stopMu.Unlock()
// A concurrent successful stop may have removed the instance while this
// caller waited for the per-instance teardown lock.
m.mu.Lock()
if current, exists := m.instances[id]; !exists || current != inst {
current, exists := m.instances[id]
if !exists {
alreadyStopped := inst.portReleased
m.mu.Unlock()
if !alreadyStopped {
return fmt.Errorf("instance %s not found", id)
}
m.logger.Debug("StopInstance already completed", zap.String("instance_id", id))
return nil
}
if current != inst {
m.mu.Unlock()
return fmt.Errorf("instance %s not found", id)
}
@@ -209,6 +209,79 @@ func TestStopInstanceRetainsPortAfterProcessTeardownFailure(t *testing.T) {
require.Equal(t, port, reusedPort)
}
func TestStopInstanceReturnsSuccessForCompletedDuplicateStop(t *testing.T) {
log := newTestLogger(t)
mgr := NewManager(&config.Config{
Ports: config.PortConfig{Base: 12345, Max: 12345},
Defaults: config.InstanceDefaults{Protocol: agent.ProtocolACP},
}, log)
t.Cleanup(func() { _ = mgr.Shutdown(context.Background()) })
port, err := mgr.portAlloc.Allocate("duplicate-stop")
require.NoError(t, err)
stopStarted := make(chan struct{})
releaseStop := make(chan struct{})
var releaseStopOnce sync.Once
releaseStopFn := func() {
releaseStopOnce.Do(func() { close(releaseStop) })
}
t.Cleanup(releaseStopFn)
procMgr := &fakeProcessManager{
stopStarted: stopStarted,
stopRelease: releaseStop,
}
inst := &Instance{
ID: "duplicate-stop",
Port: port,
Status: "running",
CreatedAt: time.Now(),
manager: procMgr,
}
mgr.mu.Lock()
mgr.instances[inst.ID] = inst
mgr.mu.Unlock()
firstDone := make(chan error, 1)
go func() { firstDone <- mgr.stopInstance(context.Background(), inst.ID, inst) }()
select {
case <-stopStarted:
case <-time.After(time.Second):
t.Fatal("first stop did not reach process teardown")
}
secondDone := make(chan error, 1)
go func() { secondDone <- mgr.stopInstance(context.Background(), inst.ID, inst) }()
releaseStopFn()
require.NoError(t, <-firstDone)
require.NoError(t, <-secondDone)
_, ok := mgr.GetInstance(inst.ID)
require.False(t, ok, "duplicate stop must leave the instance removed")
reusedPort, err := mgr.portAlloc.Allocate("replacement-instance")
require.NoError(t, err)
require.Equal(t, port, reusedPort, "duplicate stop must release the port once")
}
func TestStopInstanceRejectsReplacementForCapturedInstance(t *testing.T) {
log := newTestLogger(t)
mgr := NewManager(&config.Config{
Ports: config.PortConfig{Base: 12345, Max: 12345},
Defaults: config.InstanceDefaults{Protocol: agent.ProtocolACP},
}, log)
oldInst := &Instance{ID: "reused-id", Status: "stopping"}
newInst := &Instance{ID: "reused-id", Status: "running"}
mgr.mu.Lock()
mgr.instances[newInst.ID] = newInst
mgr.mu.Unlock()
err := mgr.stopInstance(context.Background(), oldInst.ID, oldInst)
require.ErrorContains(t, err, "instance reused-id not found")
_, ok := mgr.GetInstance(newInst.ID)
require.True(t, ok, "replacement instance must remain tracked")
require.Equal(t, "running", newInst.Info().Status)
}
func TestStopHTTPServerTreatsCanceledShutdownAsStoppedAfterClose(t *testing.T) {
log := newTestLogger(t)
mgr := NewManager(&config.Config{
@@ -241,8 +314,11 @@ func (s *fakeHTTPServer) Close() error {
}
type fakeProcessManager struct {
stopErr error
stopped bool
stopErr error
stopped bool
stopStarted chan<- struct{}
stopRelease <-chan struct{}
stopStartOnce sync.Once
}
// legacyResourceReleaseError proves a process teardown error cannot authorize
@@ -261,5 +337,11 @@ func (m *fakeProcessManager) CloseAdmission() {}
func (m *fakeProcessManager) StopForTeardown(context.Context) error {
m.stopped = true
if m.stopStarted != nil {
m.stopStartOnce.Do(func() { close(m.stopStarted) })
}
if m.stopRelease != nil {
<-m.stopRelease
}
return m.stopErr
}
@@ -108,6 +108,9 @@ type Manager struct {
stdin io.WriteCloser
stdout io.ReadCloser
stderr io.ReadCloser
stderrPipeMu sync.Mutex
stderrReader io.ReadCloser
stderrWriter io.WriteCloser
status atomic.Value // Status
exitCode atomic.Int32
exitErr atomic.Value // error
@@ -1179,12 +1182,17 @@ func (m *Manager) Start(ctx context.Context) error {
// Start the subprocess now that pipes are connected
if err := m.cmd.Start(); err != nil {
_ = m.closeStderrPipe()
m.status.Store(StatusError)
return formatAgentStartError(err, m.cfg.AgentEnv)
}
if err := m.closeStderrWriter(); err != nil {
m.logger.Debug("failed to close parent stderr pipe", zap.Error(err))
}
processLifecycle, err := installProcessLifecycle(m.cmd)
if err != nil {
reapErr := killAndWaitStartedCommand(m.cmd)
_ = m.closeStderrReader()
m.status.Store(StatusError)
return errors.Join(fmt.Errorf("failed to install agent process lifecycle: %w", err), reapErr)
}
@@ -1206,6 +1214,7 @@ func (m *Manager) Start(ctx context.Context) error {
default:
reapErr = killAndWaitStartedCommand(m.cmd)
}
_ = m.closeStderrReader()
m.status.Store(StatusError)
return errors.Join(fmt.Errorf("failed to connect adapter: %w", err), reapErr)
}
@@ -1473,15 +1482,47 @@ func (m *Manager) startProcessPipes() error {
_ = m.stdin.Close()
return fmt.Errorf("failed to create stdout pipe: %w", err)
}
m.stderr, err = m.cmd.StderrPipe()
stderrReader, stderrWriter, err := os.Pipe()
if err != nil {
_ = m.stdin.Close()
_ = m.stdout.Close()
return fmt.Errorf("failed to create stderr pipe: %w", err)
}
m.stderr = stderrReader
m.stderrPipeMu.Lock()
m.stderrReader = stderrReader
m.stderrWriter = stderrWriter
m.stderrPipeMu.Unlock()
m.cmd.Stderr = stderrWriter
return nil
}
func (m *Manager) closeStderrReader() error {
m.stderrPipeMu.Lock()
reader := m.stderrReader
m.stderrReader = nil
m.stderrPipeMu.Unlock()
if reader == nil {
return nil
}
return reader.Close()
}
func (m *Manager) closeStderrWriter() error {
m.stderrPipeMu.Lock()
writer := m.stderrWriter
m.stderrWriter = nil
m.stderrPipeMu.Unlock()
if writer == nil {
return nil
}
return writer.Close()
}
func (m *Manager) closeStderrPipe() error {
return errors.Join(m.closeStderrWriter(), m.closeStderrReader())
}
// startAgentShell auto-creates a shell session when ShellEnabled is configured.
// Failure is non-fatal: the agent can still work without a shell session.
func (m *Manager) startAgentShell() {
@@ -2303,6 +2344,7 @@ func (m *Manager) waitForStderrDrain(stderrDone <-chan struct{}) {
case <-stderrDone:
case <-timer.C:
m.logger.Warn("timed out waiting for agent stderr to drain")
_ = m.closeStderrReader()
}
}
@@ -2352,12 +2394,13 @@ func (m *Manager) waitForExit(stderrDone <-chan struct{}) {
defer close(m.doneCh)
pid := m.agentPID()
// StderrPipe must be drained before Wait closes its parent-side pipe. The
// second bounded wait covers a reader that was still blocked when the
// process-exit wait began and was released by Wait closing the pipe.
m.waitForStderrDrain(stderrDone)
// The manager owns stderr's read/write pipe, so Wait can reap the process
// without closing the reader. The reader drains the pipe concurrently.
err := m.cmd.Wait()
// Wait has observed process exit; now bound the reader drain in case a child
// process inherited the stderr writer and kept the pipe open.
m.waitForStderrDrain(stderrDone)
_ = m.closeStderrReader()
intentionalStop := m.Status() == StatusStopping
switch {
@@ -32,6 +32,169 @@ func (r *gatedStderrReader) Read(p []byte) (int, error) {
func (r *gatedStderrReader) Close() error { return nil }
type delayedStderrReader struct {
reader io.ReadCloser
started chan<- struct{}
release <-chan struct{}
startedOnce sync.Once
}
func (r *delayedStderrReader) Read(p []byte) (int, error) {
r.startedOnce.Do(func() { close(r.started) })
<-r.release
return r.reader.Read(p)
}
func (r *delayedStderrReader) Close() error { return r.reader.Close() }
func TestManagerLongRunningProcessHelper(t *testing.T) {
if os.Getenv("KANDEV_MANAGER_LONG_RUNNING_HELPER") != "1" {
return
}
select {}
}
func TestWaitForExitDoesNotWarnWhileProcessIsRunning(t *testing.T) {
log, observed := newObservedTestLogger(t)
cmd := exec.Command(os.Args[0], "-test.run=TestManagerLongRunningProcessHelper")
cmd.Env = append(os.Environ(), "KANDEV_MANAGER_LONG_RUNNING_HELPER=1")
stderr, stderrWriter, err := os.Pipe()
if err != nil {
t.Fatalf("stderr pipe: %v", err)
}
cmd.Stderr = stderrWriter
m := &Manager{
cmd: cmd,
stderr: stderr,
logger: log,
doneCh: make(chan struct{}),
updatesCh: make(chan adapter.AgentEvent, 1),
groupAliveFn: func(int) bool { return false },
}
m.status.Store(StatusRunning)
t.Cleanup(func() {
if cmd.ProcessState == nil && cmd.Process != nil {
_ = cmd.Process.Kill()
}
_ = stderrWriter.Close()
_ = stderr.Close()
m.wg.Wait()
})
if err := cmd.Start(); err != nil {
t.Fatalf("start process: %v", err)
}
if err := stderrWriter.Close(); err != nil {
t.Fatalf("close parent stderr pipe: %v", err)
}
stderrDone := make(chan struct{})
m.wg.Add(2)
go m.readStderr(stderrDone)
waitDone := make(chan struct{})
go func() {
m.waitForExit(stderrDone)
close(waitDone)
}()
timer := time.NewTimer(processStderrDrainTimeout + 100*time.Millisecond)
defer timer.Stop()
select {
case <-waitDone:
t.Fatal("waitForExit finished while the process was still running")
case <-timer.C:
if observedLogsContain(observed, "timed out waiting for agent stderr to drain") {
t.Fatal("waitForExit warned about stderr while the process was still running")
}
}
if err := cmd.Process.Kill(); err != nil {
t.Fatalf("kill process: %v", err)
}
select {
case <-waitDone:
case <-time.After(time.Second):
t.Fatal("waitForExit did not finish after process exit")
}
m.wg.Wait()
}
func TestWaitForExitPreservesStderrAfterProcessExit(t *testing.T) {
cmd := exec.Command(os.Args[0], "-test.run=TestManagerProcessExitHelper")
cmd.Env = append(os.Environ(), "KANDEV_MANAGER_PROCESS_EXIT_HELPER=1")
m := &Manager{
cmd: cmd,
logger: newTestLogger(t),
doneCh: make(chan struct{}),
updatesCh: make(chan adapter.AgentEvent, 1),
groupAliveFn: func(int) bool {
return false
},
}
m.status.Store(StatusRunning)
require.NoError(t, m.startProcessPipes())
stderrWriter, ok := cmd.Stderr.(*os.File)
if !ok {
t.Fatalf("cmd.Stderr = %T, want owned *os.File pipe", cmd.Stderr)
}
reader := m.stderr
started := make(chan struct{})
release := make(chan struct{})
m.stderr = &delayedStderrReader{
reader: reader,
started: started,
release: release,
}
t.Cleanup(func() {
select {
case <-release:
default:
close(release)
}
if cmd.ProcessState == nil && cmd.Process != nil {
_ = cmd.Process.Kill()
}
_ = stderrWriter.Close()
_ = reader.Close()
_ = m.stdin.Close()
_ = m.stdout.Close()
m.wg.Wait()
})
require.NoError(t, cmd.Start())
require.NoError(t, stderrWriter.Close())
stderrDone := make(chan struct{})
m.wg.Add(2)
go m.readStderr(stderrDone)
select {
case <-started:
case <-time.After(time.Second):
t.Fatal("stderr reader did not start")
}
waitDone := make(chan struct{})
go func() {
m.waitForExit(stderrDone)
close(waitDone)
}()
drainDelay := time.NewTimer(100 * time.Millisecond)
defer drainDelay.Stop()
<-drainDelay.C
close(release)
select {
case <-waitDone:
case <-time.After(time.Second):
t.Fatal("waitForExit did not finish after stderr drained")
}
m.wg.Wait()
event := <-m.updatesCh
recent, ok := event.Data["recent_stderr"].([]string)
if !ok {
t.Fatalf("recent_stderr = %#v", event.Data["recent_stderr"])
}
require.Equal(t, []string{"workspace=https://opencode.ai/workspace/wrk_private"}, recent)
}
func TestWaitForExitWaitsForStderrReaderBeforePublishingError(t *testing.T) {
started := make(chan struct{})
release := make(chan struct{})
@@ -48,6 +211,7 @@ func TestWaitForExitWaitsForStderrReaderBeforePublishingError(t *testing.T) {
}
m.status.Store(StatusRunning)
require.NoError(t, cmd.Start())
// m.stderr is a gated test stand-in; the subprocess writes to io.Discard.
var releaseOnce sync.Once
releaseReader := func() {
releaseOnce.Do(func() { close(release) })
@@ -116,10 +116,11 @@ func TestManagerProcessExitUsesSanitizedStderr(t *testing.T) {
log, observed := newObservedTestLogger(t)
cmd := exec.Command(os.Args[0], "-test.run=TestManagerProcessExitHelper")
cmd.Env = append(os.Environ(), "KANDEV_MANAGER_PROCESS_EXIT_HELPER=1")
stderr, err := cmd.StderrPipe()
stderr, stderrWriter, err := os.Pipe()
if err != nil {
t.Fatalf("stderr pipe: %v", err)
}
cmd.Stderr = stderrWriter
m := &Manager{
cmd: cmd,
stderr: stderr,
@@ -132,9 +133,20 @@ func TestManagerProcessExitUsesSanitizedStderr(t *testing.T) {
groupAliveFn: func(int) bool { return false },
}
m.status.Store(StatusRunning)
t.Cleanup(func() {
if cmd.ProcessState == nil && cmd.Process != nil {
_ = cmd.Process.Kill()
}
_ = stderrWriter.Close()
_ = stderr.Close()
m.wg.Wait()
})
if err := cmd.Start(); err != nil {
t.Fatalf("start process: %v", err)
}
if err := stderrWriter.Close(); err != nil {
t.Fatalf("close parent stderr pipe: %v", err)
}
stderrDone := make(chan struct{})
m.wg.Add(2)
go m.readStderr(stderrDone)
@@ -273,8 +285,11 @@ func TestStartProcessPipes_CreatesAllPipes(t *testing.T) {
assert.NotNil(t, m.stdout, "stdout pipe should be created")
assert.NotNil(t, m.stderr, "stderr pipe should be created")
// Clean up
_ = m.stdin.Close()
t.Cleanup(func() {
_ = m.stdin.Close()
_ = m.stdout.Close()
_ = m.closeStderrPipe()
})
}
func TestStartProcessPipes_FailsAfterProcessStarted(t *testing.T) {
+5 -2
View File
@@ -20,7 +20,7 @@ var (
// Settings-triggered restart rebuilds from source) plus the Vite dev server.
// All dev state lives under <repoRoot>/.kandev-dev/ so `make dev` never
// mutates the user's production ~/.kandev.
func runDev(ctx context.Context, opts Options) int {
func runDev(ctx context.Context, opts Options, build BuildInfo) int {
cfg, code := devLaunchConfigFor(opts)
if code != 0 {
return code
@@ -29,7 +29,7 @@ func runDev(ctx context.Context, opts Options) int {
fmt.Fprintln(os.Stderr, "[kandev] "+err.Error())
return 1
}
logStartup("dev mode: using local repo", cfg.ports, cfg.dbPath, cfg.logLevel, serverHostForConfig(cfg.startup))
logStartup("dev mode: using local repo", build.Version, cfg.ports, cfg.dbPath, cfg.logLevel, serverHostForConfig(cfg.startup))
ignoreBrokenPipeSignal()
setLauncherShutdownDebug(cfg.debug || os.Getenv("KANDEV_SHUTDOWN_DEBUG") == "1")
@@ -77,7 +77,10 @@ func runDev(ctx context.Context, opts Options) int {
return 1
}
fmt.Printf("[kandev] backend ready at %s\n", cfg.ports.BackendURL)
return runDevWeb(ctx, opts, cfg, supervisor, backend)
}
func runDevWeb(ctx context.Context, opts Options, cfg devLaunchConfig, supervisor *processSupervisor, backend *restartableBackend) int {
webURL := fmt.Sprintf("http://localhost:%d", cfg.ports.WebPort)
fmt.Println("[kandev] starting web...")
webProc, err := launchWebChild(cfg.repoRoot, cfg.ports, supervisor)
+15 -9
View File
@@ -16,7 +16,7 @@ func itoa(n int) string { return strconv.Itoa(n) }
var errTestHealth = errors.New("test health failure")
func TestRunDevLaunchesBackendThenWebAndOpensBrowser(t *testing.T) {
func TestRunDevBuildVersionAndLaunchesBackendThenWebAndOpensBrowser(t *testing.T) {
repo := makeRepoTree(t)
t.Chdir(repo)
t.Setenv("KANDEV_TASK_ID", "")
@@ -89,10 +89,16 @@ func TestRunDevLaunchesBackendThenWebAndOpensBrowser(t *testing.T) {
return &managedProcess{}, func() {}, nil
}
code := runDev(context.Background(), Options{Command: CommandDev})
var code int
output := captureLauncherStdout(t, func() {
code = runDev(context.Background(), Options{Command: CommandDev}, BuildInfo{Version: "1.2.3"})
})
if code != 0 {
t.Fatalf("runDev() = %d, want 0", code)
}
if !strings.Contains(output, "[kandev] version: 1.2.3\n") {
t.Fatalf("startup output = %q, missing build version", output)
}
want := []string{
"new-supervisor",
"attach-signals",
@@ -212,7 +218,7 @@ func TestRunDevUsesConfiguredHealthTimeoutForBackendAndWeb(t *testing.T) {
return &managedProcess{}, func() {}, nil
}
if code := runDev(context.Background(), Options{Command: CommandDev}); code != 0 {
if code := runDev(context.Background(), Options{Command: CommandDev}, BuildInfo{}); code != 0 {
t.Fatalf("runDev() = %d, want 0", code)
}
want := 12345 * time.Millisecond
@@ -266,7 +272,7 @@ func TestRunDevHeadlessSkipsBrowser(t *testing.T) {
return &managedProcess{}, func() {}, nil
}
code := runDev(context.Background(), Options{Command: CommandDev, Headless: true})
code := runDev(context.Background(), Options{Command: CommandDev, Headless: true}, BuildInfo{})
if code != 0 {
t.Fatalf("runDev(headless) = %d, want 0", code)
}
@@ -296,7 +302,7 @@ func TestRunDevAbortsOnBackupFailureBeforeLaunch(t *testing.T) {
return nil, nil, nil
}
code := runDev(context.Background(), Options{Command: CommandDev})
code := runDev(context.Background(), Options{Command: CommandDev}, BuildInfo{})
if code != 1 {
t.Fatalf("runDev() = %d, want 1", code)
}
@@ -350,7 +356,7 @@ func TestRunDevShutsDownTreeOnBackendHealthFailure(t *testing.T) {
return nil, nil, nil
}
code := runDev(context.Background(), Options{Command: CommandDev})
code := runDev(context.Background(), Options{Command: CommandDev}, BuildInfo{})
if code != 1 {
t.Fatalf("runDev() = %d, want 1", code)
}
@@ -415,7 +421,7 @@ func TestRunDevShutsDownTreeOnBackendReadinessFailure(t *testing.T) {
return nil, nil, nil
}
code := runDev(context.Background(), Options{Command: CommandDev})
code := runDev(context.Background(), Options{Command: CommandDev}, BuildInfo{})
if code != 1 {
t.Fatalf("runDev() = %d, want 1", code)
}
@@ -480,7 +486,7 @@ func TestRunDevShutsDownTreeWhenWebChildExits(t *testing.T) {
// signal waitForAppExit selects on.
close(webDone)
code := runDev(context.Background(), Options{Command: CommandDev, Headless: true})
code := runDev(context.Background(), Options{Command: CommandDev, Headless: true}, BuildInfo{})
if code != 7 {
t.Fatalf("runDev() = %d, want the web child's exit code 7", code)
}
@@ -492,7 +498,7 @@ func TestRunDevShutsDownTreeWhenWebChildExits(t *testing.T) {
func TestRunDevFailsWhenRepoRootNotFound(t *testing.T) {
t.Chdir(t.TempDir())
code := runDev(context.Background(), Options{Command: CommandDev})
code := runDev(context.Background(), Options{Command: CommandDev}, BuildInfo{})
if code != 2 {
t.Fatalf("runDev() = %d, want 2", code)
}
+4 -3
View File
@@ -23,6 +23,7 @@ func Run(args []string, build BuildInfo) int {
fmt.Fprintln(os.Stderr, "[kandev] "+err.Error())
return 2
}
build.Version = normalizedBuildVersion(build.Version)
if opts.ShowVersion {
fmt.Println(build.Version)
return 0
@@ -36,11 +37,11 @@ func Run(args []string, build BuildInfo) int {
ctx := context.Background()
switch opts.Command {
case CommandStart:
return runStart(ctx, opts)
return runStart(ctx, opts, build)
case CommandRun:
return runInstalled(ctx, opts)
return runInstalled(ctx, opts, build)
case CommandDev:
return runDev(ctx, opts)
return runDev(ctx, opts, build)
}
fmt.Fprintf(os.Stderr, "[kandev] native launcher command %q is not implemented yet\n", opts.Command)
return 1
@@ -36,7 +36,7 @@ func TestLogStartupPrintsNetworkAddress(t *testing.T) {
}
output := captureLauncherStdout(t, func() {
logStartup("test", portConfig{
logStartup("test", "", portConfig{
BackendPort: 38429,
BackendURL: "http://localhost:38429",
}, "", "")
@@ -228,7 +228,7 @@ func TestLogStartupKeepsLocalhostWhenNetworkDiscoveryFails(t *testing.T) {
}
output := captureLauncherStdout(t, func() {
logStartup("test", portConfig{
logStartup("test", "", portConfig{
BackendPort: 38429,
BackendURL: "http://localhost:38429",
}, "", "")
@@ -257,7 +257,7 @@ func TestLogStartupSuppressesNetworkAddressesForLoopbackBind(t *testing.T) {
}
output := captureLauncherStdout(t, func() {
logStartup("test", portConfig{
logStartup("test", "", portConfig{
BackendPort: 38429,
BackendURL: "http://localhost:38429",
}, "", "")
+2 -1
View File
@@ -7,7 +7,7 @@ import (
"path/filepath"
)
func runInstalled(ctx context.Context, opts Options) int {
func runInstalled(ctx context.Context, opts Options, build BuildInfo) int {
startupConfig, err := loadBootstrapConfig()
if err != nil {
fmt.Fprintln(os.Stderr, "[kandev] "+err.Error())
@@ -46,6 +46,7 @@ func runInstalled(ctx context.Context, opts Options) int {
}
return launchManaged(ctx, managedAppConfig{
Header: "release: " + releaseTag,
Version: normalizedBuildVersion(build.Version),
Mode: "run",
Backend: bundle.Launcher,
BackendCWD: filepath.Dir(bundle.Launcher),
+13 -3
View File
@@ -28,7 +28,7 @@ var (
}
)
func runStart(ctx context.Context, opts Options) int {
func runStart(ctx context.Context, opts Options, build BuildInfo) int {
startupConfig, err := loadBootstrapConfig()
if err != nil {
fmt.Fprintln(os.Stderr, "[kandev] "+err.Error())
@@ -64,6 +64,7 @@ func runStart(ctx context.Context, opts Options) int {
}
return launchManaged(ctx, managedAppConfig{
Header: "start mode: using local build",
Version: normalizedBuildVersion(build.Version),
Mode: "start",
Backend: self,
BackendCWD: filepath.Dir(self),
@@ -77,6 +78,7 @@ func runStart(ctx context.Context, opts Options) int {
type managedAppConfig struct {
Header string
Version string
Mode string
Backend string
BackendCWD string
@@ -131,7 +133,7 @@ func runManagedApp(ctx context.Context, cfg managedAppConfig) int {
}
cfg.Ports.BackendURL = cfg.Endpoints.accessURL
ignoreBrokenPipeSignal()
logStartup(cfg.Header, cfg.Ports, resolveDatabasePathForConfig(cfg.Startup), cfg.LogLevel, serverHostForConfig(cfg.Startup))
logStartup(cfg.Header, cfg.Version, cfg.Ports, resolveDatabasePathForConfig(cfg.Startup), cfg.LogLevel, serverHostForConfig(cfg.Startup))
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)
@@ -191,8 +193,9 @@ func runManagedApp(ctx context.Context, cfg managedAppConfig) int {
return waitForAppExit(supervisor, backend)
}
func logStartup(header string, ports portConfig, dbPath, logLevel string, serverHosts ...string) {
func logStartup(header, version string, ports portConfig, dbPath, logLevel string, serverHosts ...string) {
fmt.Println("[kandev] " + header)
fmt.Println("[kandev] version:", normalizedBuildVersion(version))
fmt.Println("[kandev] url:", ports.BackendURL)
serverHost := os.Getenv("KANDEV_SERVER_HOST")
if len(serverHosts) > 0 && serverHosts[0] != "" {
@@ -211,6 +214,13 @@ func logStartup(header string, ports portConfig, dbPath, logLevel string, server
}
}
func normalizedBuildVersion(version string) string {
if version == "" {
return "dev"
}
return version
}
func openBrowser(url string) {
if os.Getenv("KANDEV_NO_BROWSER") == "1" {
return
+2 -2
View File
@@ -144,7 +144,7 @@ func TestRunStartUsesSelfExecutableAndBackendCWD(t *testing.T) {
// dir must be canonical (macOS temp dirs sit under the /var -> /private/var link).
t.Setenv("KANDEV_HOME_DIR", canonicalTempDir(t))
code := runStart(context.Background(), Options{Command: CommandStart, BackendPort: 48123, Headless: true})
code := runStart(context.Background(), Options{Command: CommandStart, BackendPort: 48123, Headless: true}, BuildInfo{})
if code != 42 {
t.Fatalf("runStart() = %d, want 42", code)
}
@@ -187,7 +187,7 @@ func TestRunStartUsesConfiguredBindForBackendURL(t *testing.T) {
return 42
}
if code := runStart(context.Background(), Options{Command: CommandStart, Headless: true}); code != 42 {
if code := runStart(context.Background(), Options{Command: CommandStart, Headless: true}, BuildInfo{}); code != 42 {
t.Fatalf("runStart() = %d, want 42", code)
}
if got.Ports.BackendURL != "http://192.0.2.10:48123" {
@@ -0,0 +1,110 @@
package launcher
import (
"context"
"path/filepath"
"strings"
"testing"
)
func TestLogStartupPrintsVersion(t *testing.T) {
t.Setenv("KANDEV_SERVER_HOST", "127.0.0.1")
output := captureLauncherStdout(t, func() {
logStartup("test", "1.2.3", portConfig{
BackendPort: 38429,
BackendURL: "http://localhost:38429",
}, "", "")
})
if !strings.Contains(output, "[kandev] version: 1.2.3\n") {
t.Fatalf("startup output = %q, missing version", output)
}
if strings.Index(output, "[kandev] version:") > strings.Index(output, "[kandev] url:") {
t.Fatalf("startup output = %q, version must precede URL", output)
}
}
func TestLogStartupDefaultsEmptyVersionToDev(t *testing.T) {
t.Setenv("KANDEV_SERVER_HOST", "127.0.0.1")
output := captureLauncherStdout(t, func() {
logStartup("test", "", portConfig{
BackendPort: 38429,
BackendURL: "http://localhost:38429",
}, "", "")
})
if got := strings.Count(output, "[kandev] version:"); got != 1 {
t.Fatalf("startup output = %q, version line count = %d, want 1", output, got)
}
if !strings.Contains(output, "[kandev] version: dev\n") {
t.Fatalf("startup output = %q, want dev fallback", output)
}
}
func TestRunUsesDevVersionForEmptyBuild(t *testing.T) {
output := captureLauncherStdout(t, func() {
if code := Run([]string{"--version"}, BuildInfo{}); code != 0 {
t.Fatalf("Run(--version) = %d, want 0", code)
}
})
if output != "dev\n" {
t.Fatalf("--version output = %q, want %q", output, "dev\n")
}
}
func TestRunStartBuildVersion(t *testing.T) {
oldExecutablePath := executablePath
oldLaunchManaged := launchManaged
t.Cleanup(func() {
executablePath = oldExecutablePath
launchManaged = oldLaunchManaged
})
exe := filepath.Join(canonicalTempDir(t), "bin", "kandev")
executablePath = func() (string, error) {
return exe, nil
}
t.Setenv("KANDEV_HOME_DIR", canonicalTempDir(t))
var got managedAppConfig
launchManaged = func(_ context.Context, cfg managedAppConfig) int {
got = cfg
return 42
}
if code := runStart(context.Background(), Options{Command: CommandStart, Headless: true}, BuildInfo{Version: "1.2.3"}); code != 42 {
t.Fatalf("runStart() = %d, want 42", code)
}
if got.Version != "1.2.3" {
t.Fatalf("managed startup version = %q, want 1.2.3", got.Version)
}
}
func TestRunInstalledBuildVersion(t *testing.T) {
oldLaunchManaged := launchManaged
t.Cleanup(func() { launchManaged = oldLaunchManaged })
bundle := t.TempDir()
writeFile(t, filepath.Join(bundle, "bin", executableName("kandev")))
writeFile(t, filepath.Join(bundle, "bin", executableName("agentctl")))
writeRemoteAgentctlHelpers(t, bundle)
t.Setenv("KANDEV_BUNDLE_DIR", bundle)
clearLauncherConfigurationEnvironment(t)
t.Chdir(t.TempDir())
homeDir := canonicalTempDir(t)
t.Setenv("KANDEV_HOME_DIR", homeDir)
var got managedAppConfig
launchManaged = func(_ context.Context, cfg managedAppConfig) int {
got = cfg
return 42
}
if code := runInstalled(context.Background(), Options{Command: CommandRun, Headless: true}, BuildInfo{Version: "1.2.3"}); code != 42 {
t.Fatalf("runInstalled() = %d, want 42", code)
}
if got.Version != "1.2.3" {
t.Fatalf("managed startup version = %q, want 1.2.3", got.Version)
}
}
@@ -0,0 +1,124 @@
---
created: 2026-08-26
status: in_progress
requirements:
- REQ-PLATFORM-GO-DEV-LAUNCHER-001
- REQ-PLATFORM-AGENT-PROCESS-EXIT-DRAIN-001
- REQ-PLATFORM-AGENTCTL-INSTANCE-STOP-001
system_design:
- ../../specs/platform/system-design/go-dev-launcher.md
- ../../specs/platform/system-design/agent-process-exit-drain.md
- ../../specs/platform/system-design/agentctl-instance-stop.md
legacy_specs: []
---
# Implementation Plan: Startup observability and lifecycle cleanup
## Overview
The 2026-08-26 log audit found two confirmed implementation defects and one
requested observability gap. First, `waitForExit` emits a false stderr-drain
warning because it waits for EOF before `cmd.Wait`. Second, duplicate instance
stops return HTTP 500 after another stop has already completed. Third, the
native launcher does not print the build version in its startup banner. The
work orders are sequential because each changes a lifecycle boundary and needs
focused regression evidence before the next change is reviewed.
## Scope
### In scope
- Print the native build version in `dev`, `start`, and `run` startup banners.
- Make stderr-drain timeout warnings post-exit diagnostics only.
- Make overlapping stops for the same agentctl instance converge on the stopped
state without masking genuine teardown failures.
- Update the public CLI reference for the new startup line.
### Out of scope
- Cursor ACP authentication failure. This requires the operator to log in or
remove the unready Cursor agent configuration.
- SSE MCP filtering. This is expected capability negotiation for ACP agents
that do not support SSE.
- Shell stop grace periods, process-group force-kill policy, Git polling policy,
and log rotation.
- Broad reclassification of unrelated agentctl stderr or startup warnings.
## Technical approach
### Startup version identity
Pass `launcher.BuildInfo.Version` from `launcher.Run` through the mode-specific
launch configuration into the shared `logStartup` function. Render one
`[kandev] version:` line before the existing URL lines, using the native `dev`
default when metadata is empty. Keep `KANDEV_VERSION` as optional installed
release header metadata. Update `docs/public/cli.md` in the same implementation
change.
### Agent stderr exit sequencing
Keep `readStderr` running concurrently with the child process. Replace the
manager's `Cmd.StderrPipe` with an explicitly owned `os.Pipe`, then change
`internal/agentctl/server/process.Manager.waitForExit` to call `cmd.Wait()`
before the bounded wait on the generation-specific stderr completion channel.
Close the owned reader when the bounded drain expires. Preserve recent-stderr
sanitization, exit events, intentional-stop behavior, and process-group
reaping.
### Idempotent instance stop
Keep the initial unknown-ID 404 and the existing per-instance `stopMu`. After a
stop caller acquires the lock, distinguish removal of the same instance pointer
after successful teardown from an ID remapped to a different instance. Treat
the former as an already-satisfied stop, release the port only once, and retain
HTTP 500 plus the instance for genuine cleanup failures.
## Tests
- `AC-PLATFORM-GO-DEV-LAUNCHER-001.9`: add launcher banner coverage in
`apps/backend/internal/launcher/network_test.go` or a focused startup test,
including stamped, empty, and `--version`-equivalent values.
- `AC-PLATFORM-AGENT-PROCESS-EXIT-DRAIN-001.1` through `.4`: extend
`apps/backend/internal/agentctl/server/process/manager_stderr_exit_test.go`
to prove no pre-exit timeout warning, post-exit bounded drain, preserved
recent stderr, and unchanged intentional/error exit paths.
- `AC-PLATFORM-AGENTCTL-INSTANCE-STOP-001.1` and `.4`: add deterministic
concurrent-stop coverage in
`apps/backend/internal/agentctl/server/instance/manager_shutdown_test.go`.
- `AC-PLATFORM-AGENTCTL-INSTANCE-STOP-001.2` and `.3`: retain or extend
control-server boundary coverage for unknown IDs and cleanup failures.
## E2E tests
The new version contract is native launcher stdout, not browser behavior, so a
Playwright project cannot observe it. The package uses an end-to-end launcher
boundary test in `apps/backend/internal/launcher` and the existing backend
process tests for the two internal lifecycle contracts. No browser E2E test is
needed for these acceptance criteria.
## Work orders
- [x] [Task 01: Startup banner version identity](task-01-startup-version.md) (done)
- [x] [Task 02: Agent stderr exit sequencing](task-02-agent-stderr-exit.md) (done)
- [x] [Task 03: Idempotent agent instance stop](task-03-instance-stop.md) (done)
## Verification results
- Task 01 focused launcher tests passed.
- Public documentation validation passed (`node --test scripts/validate-public-docs.test.mjs`; `node scripts/validate-public-docs.mjs`).
- The full launcher package ran 259 tests successfully, with two pre-existing
service configuration tests selecting the host's `/root/.kandev/config.yaml`
instead of their temporary fixture.
- The full process-manager package passed 707 tests after the stderr sequencing
fix and owned-pipe correction.
- The combined instance and control-server packages passed 463 tests after the
idempotent-stop fix.
## Risks
- The owned stderr writer must close in the parent after startup, while the
reader remains concurrent so a child that writes a full pipe cannot deadlock.
- Treating a missing map entry as already stopped must be tied to the original
instance pointer so an ID reuse cannot be mistaken for completed cleanup.
- The startup banner is consumed by people and scripts, so existing line order
and values must remain stable apart from the additive version line.
@@ -0,0 +1,94 @@
---
id: "01-startup-version"
title: "Startup banner version identity"
status: done
wave: 1
depends_on: []
plan: "plan.md"
requirements:
- REQ-PLATFORM-GO-DEV-LAUNCHER-001
acceptance_criteria:
- AC-PLATFORM-GO-DEV-LAUNCHER-001.9
system_design:
- ../../specs/platform/system-design/go-dev-launcher.md
---
# Task 01: Startup banner version identity
## Summary
Add the native build version to the shared `dev`, `start`, and `run` startup
banner. Preserve the existing startup fields and document the new line in the
public CLI reference.
## In scope
- Carry `launcher.BuildInfo.Version` into each shared startup path.
- Render the `version:` line before the existing URL lines.
- Use `dev` for an unstamped or empty build value.
- Add focused launcher coverage and update `docs/public/cli.md`.
## Out of scope
- Changing service-unit version metadata.
- Changing `KANDEV_VERSION` release-header behavior.
- Changing backend health or system-info payloads.
## Acceptance
- Every `dev`, `start`, and `run` startup banner includes exactly one version
line whose value matches the binary's `--version` value.
- Empty build metadata renders `dev`, and all existing URL, MCP, database, and
log-level lines remain present and unchanged.
- The public CLI reference describes the additive version line and its source.
## Verification
```bash
cd apps/backend && go test ./internal/launcher -run 'TestLogStartupPrintsVersion|TestRun.*BuildVersion' -count=1
cd apps/backend && go test ./internal/launcher -count=1
node --test scripts/validate-public-docs.test.mjs
node scripts/validate-public-docs.mjs
```
## Files likely touched
- `apps/backend/internal/launcher/launcher.go`
- `apps/backend/internal/launcher/start.go`
- `apps/backend/internal/launcher/run.go`
- `apps/backend/internal/launcher/dev.go`
- `apps/backend/internal/launcher/network_test.go`
- `docs/public/cli.md`
## Dependencies
None.
## Risks
- Direct launcher tests construct mode configuration values, so the build
version must be threaded without changing unrelated startup defaults.
## Parallelism
`sequential`
## Inputs
- `docs/specs/platform/requirements/go-dev-launcher.md`, AC `.9`.
- `docs/specs/platform/system-design/go-dev-launcher.md`.
- Existing `launcher.BuildInfo`, `logStartup`, and network banner tests.
## Results
Implemented the version line for `dev`, `start`, and `run`. The value is
normalized to `dev` when build metadata is empty and is shared with the
launcher `--version` path. Updated the public CLI reference.
Verification:
- `go test ./internal/launcher -run 'TestLogStartupPrintsVersion|TestRun.*BuildVersion' -count=1` passed.
- `go test ./internal/launcher -count=1` ran 259 tests; two existing service
configuration tests were blocked by the host's `/root/.kandev/config.yaml`.
- `node --test scripts/validate-public-docs.test.mjs` passed.
- `node scripts/validate-public-docs.mjs` passed.
@@ -0,0 +1,94 @@
---
id: "02-agent-stderr-exit"
title: "Agent stderr exit sequencing"
status: done
wave: 2
depends_on:
- "01-startup-version"
plan: "plan.md"
requirements:
- REQ-PLATFORM-AGENT-PROCESS-EXIT-DRAIN-001
acceptance_criteria:
- AC-PLATFORM-AGENT-PROCESS-EXIT-DRAIN-001.1
- AC-PLATFORM-AGENT-PROCESS-EXIT-DRAIN-001.2
- AC-PLATFORM-AGENT-PROCESS-EXIT-DRAIN-001.3
- AC-PLATFORM-AGENT-PROCESS-EXIT-DRAIN-001.4
system_design:
- ../../specs/platform/system-design/agent-process-exit-drain.md
---
# Task 02: Agent stderr exit sequencing
## Summary
Correct the process-manager exit order so a long-running agent does not produce
a false stderr-drain timeout warning. Keep the bounded post-exit drain and all
existing sanitized exit diagnostics.
## In scope
- Replace the manager's `Cmd.StderrPipe` with an explicitly owned stderr pipe.
- Change `Manager.waitForExit` to wait for process exit before stderr drain.
- Preserve generation-specific completion channels and the bounded timeout.
- Add regression coverage for running, successful, intentional-stop, and
non-zero-exit paths.
## Out of scope
- Changing the drain timeout value.
- Changing stderr sanitization, redaction, or ring-buffer limits.
- Changing process-group cleanup or ACP adapter behavior.
## Acceptance
- No stderr-drain timeout is emitted while a managed process is still running.
- Exit publication waits for the generation-specific stderr reader after
`cmd.Wait`, but remains bounded when the reader is held open.
- Exit code, error event, safe recent stderr, intentional-stop classification,
and process-group cleanup remain unchanged.
## Verification
```bash
cd apps/backend && go test ./internal/agentctl/server/process -run 'TestWaitForExit.*|TestReadStderr.*Generation' -count=1
cd apps/backend && go test ./internal/agentctl/server/process -count=1
```
## Files likely touched
- `apps/backend/internal/agentctl/server/process/manager.go`
- `apps/backend/internal/agentctl/server/process/manager_stderr_exit_test.go`
## Dependencies
Task 01 is a prior plan wave. The code paths are independent, but the plan
keeps lifecycle changes sequential for review.
## Risks
- The manager must close its parent stderr writer after start and close its
reader if the bounded post-exit drain expires.
## Parallelism
`sequential`
## Inputs
- `docs/specs/platform/requirements/agent-process-exit-drain.md`.
- `docs/specs/platform/system-design/agent-process-exit-drain.md`.
- Existing generation-channel and sanitized-stderr tests.
## Results
Changed the manager to use an explicitly owned stderr pipe, then wait for the
child process before waiting on the generation-specific stderr reader. Added
regression coverage proving a live process does not emit the drain timeout
warning, exit diagnostics preserve stderr after `Cmd.Wait`, and the reader
drain remains bounded.
Verification:
- `go test ./internal/agentctl/server/process -run 'TestWaitForExit.*|TestReadStderr.*Generation' -count=1` passed.
- `go test ./internal/agentctl/server/process -count=1` passed with 707 tests.
- Race-enabled stderr exit tests passed, including owned-pipe preservation.
@@ -0,0 +1,94 @@
---
id: "03-instance-stop"
title: "Idempotent agent instance stop"
status: done
wave: 3
depends_on:
- "02-agent-stderr-exit"
plan: "plan.md"
requirements:
- REQ-PLATFORM-AGENTCTL-INSTANCE-STOP-001
acceptance_criteria:
- AC-PLATFORM-AGENTCTL-INSTANCE-STOP-001.1
- AC-PLATFORM-AGENTCTL-INSTANCE-STOP-001.2
- AC-PLATFORM-AGENTCTL-INSTANCE-STOP-001.3
- AC-PLATFORM-AGENTCTL-INSTANCE-STOP-001.4
system_design:
- ../../specs/platform/system-design/agentctl-instance-stop.md
---
# Task 03: Idempotent agent instance stop
## Summary
Make an overlapping stop call succeed when the same instance has already been
fully removed by another stop operation. Keep unknown-instance responses and
real cleanup failures distinct.
## In scope
- Serialize and recheck the original instance pointer after `stopMu`.
- Treat removal of that same pointer as an already-satisfied stop.
- Release ports once and retain the current failure retry behavior.
- Add deterministic manager and control-boundary regression tests.
## Out of scope
- Changing the `ControlClient` lost-response retry contract.
- Changing process or HTTP-server grace periods.
- Changing instance persistence, IDs, or agent protocol behavior.
## Acceptance
- Overlapping stops for one instance do not produce a 500 solely because the
first stop removed the instance.
- Unknown IDs remain non-mutating 404 responses, and a different instance
reusing the ID is never mistaken for the original instance.
- Genuine cleanup failures remain HTTP 500/error-level and retain the instance
and port for retry; successful cleanup releases the port once.
## Verification
```bash
cd apps/backend && go test ./internal/agentctl/server/instance ./internal/agentctl/server/api -count=1
```
## Files likely touched
- `apps/backend/internal/agentctl/server/instance/manager.go`
- `apps/backend/internal/agentctl/server/instance/manager_shutdown_test.go`
- `apps/backend/internal/agentctl/server/api/control_server.go`
- `apps/backend/internal/agentctl/server/api/*_test.go`
## Dependencies
Task 02 is a prior plan wave. No data migration or external service is
required.
## Risks
- A missing map entry is only benign when it follows teardown of the exact
pointer captured before waiting; an ID reuse must remain an error.
## Parallelism
`sequential`
## Inputs
- `docs/specs/platform/requirements/agentctl-instance-stop.md`.
- `docs/specs/platform/system-design/agentctl-instance-stop.md`.
- Existing `Manager.StopInstance` failure-retention tests and
`ControlClient.DeleteInstance` 404 retry test.
## Results
Refactored stop handling so the initial instance pointer is passed through a
serialized stop operation. A pointer that was already removed by a successful
stop now returns success, while a replacement pointer still returns not found.
Added manager concurrency, port-reuse, replacement-safety, and unknown-control-
route coverage.
Verification:
- `go test ./internal/agentctl/server/instance ./internal/agentctl/server/api -count=1` passed with 463 tests.
+2
View File
@@ -231,6 +231,8 @@ The launcher derives readiness targets and the access URL from the effective `se
When it can enumerate non-loopback interfaces, the launcher also prints `network:` URLs for each unique non-loopback, non-link-local address on the same port, including local-network and Tailscale addresses. IPv6 addresses are shown in brackets. If the effective bind restricts the listener, the launcher only prints matching addresses. These lines are informational and are omitted if interface discovery is unavailable.
The `dev`, `start`, and `run` startup banners also print a `version:` line. Its value matches `kandev --version`; an unstamped local build reports `dev`.
The backend's default `server.host` is `0.0.0.0`. That can expose Kandev to other machines on the network, and the current local product path is not an authenticated multi-user boundary. The launcher uses `localhost` for its default local access URL, but the backend still listens on every interface. Bind it to loopback unless remote access is deliberately protected:
```bash
+5
View File
@@ -34,6 +34,8 @@ localization, feature toggles, health, and shared session recovery services.
- [Agent process exit and stderr drain](requirements/agent-process-exit-drain.md)
- [Agentctl instance stop idempotency](requirements/agentctl-instance-stop.md)
- [Agent Runtime Availability](requirements/agent-runtime-availability.md)
- [Background Work Liveness](requirements/background-work-liveness.md)
- [Bounded Task Status Delivery](requirements/bounded-task-status-delivery.md)
@@ -70,6 +72,8 @@ localization, feature toggles, health, and shared session recovery services.
- [Agent process exit and stderr drain](system-design/agent-process-exit-drain.md)
- [Agentctl instance stop idempotency](system-design/agentctl-instance-stop.md)
- [Browser console retention](system-design/browser-console-retention.md)
- [Bounded Task Status Delivery](system-design/bounded-task-status-delivery.md)
- [Diagnostic logging System Design Part 1](system-design/diagnostic-logging-01.md)
@@ -81,6 +85,7 @@ localization, feature toggles, health, and shared session recovery services.
- [Session MCP Attachment Observability](system-design/mcp-session-observability.md)
- [Provider Error Recovery](system-design/provider-error-recovery.md)
- [Workspace Git Status](system-design/workspace-git-status.md)
- [Go dev launcher and startup version](system-design/go-dev-launcher.md)
## Migration record
@@ -0,0 +1,44 @@
---
status: draft
system: platform
created: 2026-08-26
owners:
- kandev
---
# Agent process exit and stderr drain Requirements
## Overview
The agent process manager reads child stderr while it supervises process exit.
The current order waits for stderr EOF before waiting for the child process. A
long-running agent therefore produces a false stderr-drain warning after one
second, even though the agent is still starting normally. This requirement keeps
exit diagnostics complete without making process shutdown unbounded.
## Terminology
- **stderr drain:** Completion of the generation-specific reader that consumes a
managed agent process's stderr pipe.
- **exit publication:** The process-manager state and update event that expose a
managed agent's final exit result.
## Requirements
### REQ-PLATFORM-AGENT-PROCESS-EXIT-DRAIN-001: Correct agent process exit drain sequencing
**Intent:** Make the stderr-drain warning describe a real post-exit cleanup delay,
while preserving safe recent-stderr diagnostics and a bounded shutdown path.
#### Acceptance criteria
- **AC-PLATFORM-AGENT-PROCESS-EXIT-DRAIN-001.1:** When a managed agent remains running longer than the stderr-drain timeout, the process manager shall not emit a stderr-drain timeout warning solely because the process is still running.
- **AC-PLATFORM-AGENT-PROCESS-EXIT-DRAIN-001.2:** When a managed agent exits, the process manager shall wait for its generation-specific stderr reader, up to the existing bounded timeout, before publishing the final exit result.
- **AC-PLATFORM-AGENT-PROCESS-EXIT-DRAIN-001.3:** When the stderr reader remains blocked after the managed process exits, the process manager shall emit a warning and continue exit handling within the existing bounded timeout.
- **AC-PLATFORM-AGENT-PROCESS-EXIT-DRAIN-001.4:** The exit code, error classification, safe recent-stderr content, process-group cleanup, and intentional-stop behavior shall remain unchanged.
## Out of scope
- Changing the stderr timeout duration.
- Changing stderr sanitization or the recent-stderr buffer contents.
- Changing ACP capability filtering, agent authentication, shell grace periods,
or process-group termination policy.
@@ -0,0 +1,46 @@
---
status: draft
system: platform
created: 2026-08-26
owners:
- kandev
---
# Agentctl instance stop idempotency Requirements
## Overview
The agentctl control API can receive a second stop request while the first
request is finishing teardown. The instance manager removes the instance after
successful cleanup, so the second request currently reports a 500 even though
the requested stopped state is already true. This requirement makes repeated
stops converge on the same safe postcondition while preserving real cleanup
failures.
## Terminology
- **Already stopped:** The requested instance was successfully torn down and
removed by another stop operation before this operation completed.
- **Real cleanup failure:** HTTP-server or process-manager cleanup failed, so
the instance or its port must remain available for retry.
## Requirements
### REQ-PLATFORM-AGENTCTL-INSTANCE-STOP-001: Idempotent agentctl instance stop
**Intent:** Prevent a teardown race from being reported as an internal failure
after the instance has already been stopped, without hiding an incomplete
cleanup operation.
#### Acceptance criteria
- **AC-PLATFORM-AGENTCTL-INSTANCE-STOP-001.1:** When a `DELETE /api/v1/instances/:id` request races with another stop for the same tracked instance and the other stop completes first, the request shall not return HTTP 500 solely because that instance is already stopped; the completed-stop outcome may be HTTP 200 or HTTP 404.
- **AC-PLATFORM-AGENTCTL-INSTANCE-STOP-001.2:** When a stop request observes an unknown instance that was not part of a completed stop, the control API shall retain its HTTP 404 response and shall not release an unrelated port or instance.
- **AC-PLATFORM-AGENTCTL-INSTANCE-STOP-001.3:** When HTTP-server or process-manager cleanup fails, the control API shall retain its HTTP 500 response and error-level diagnostic, and the instance and its allocated port shall remain retryable.
- **AC-PLATFORM-AGENTCTL-INSTANCE-STOP-001.4:** When cleanup succeeds, the instance shall be removed from tracking and its port shall be released at most once, including when duplicate stop calls overlap.
## Out of scope
- Changing the existing `ControlClient` treatment of HTTP 404 after a lost
delete response.
- Changing process termination grace periods or agent protocol behavior.
- Adding persistence for stopped instances.
@@ -27,6 +27,7 @@ owners:
- **AC-PLATFORM-GO-DEV-LAUNCHER-001.6:** **GIVEN** a host with an IPv6 address, **WHEN** the startup banner is printed, **THEN** the address is rendered as `http://[<address>]:<port>`.
- **AC-PLATFORM-GO-DEV-LAUNCHER-001.7:** **GIVEN** interface enumeration returns an error, **WHEN** the launcher starts, **THEN** it continues startup and prints the localhost URL without a `network:` line.
- **AC-PLATFORM-GO-DEV-LAUNCHER-001.8:** **GIVEN** `KANDEV_SERVER_HOST` is a loopback or specific bind address, **WHEN** the startup banner is printed, **THEN** it omits unrelated network addresses.
- **AC-PLATFORM-GO-DEV-LAUNCHER-001.9:** **WHEN** the `dev`, `start`, or `run` launcher prints its startup banner, it shall include a `version:` line whose value is the same build version printed by `--version` for that binary. An unstamped build shall print `dev`.
## Migrated source detail
@@ -142,6 +143,13 @@ cannot enumerate its interfaces, the launcher still starts and prints the localh
URL. When `KANDEV_SERVER_HOST` restricts the backend to loopback or specific addresses,
the banner omits unreachable interface addresses and only advertises matching binds.
### Startup output advertises the build version
The startup banner for `dev`, `start`, and `run` also prints a `version:` line. Its
value is the launcher build version, the same value returned by `--version`; an
unstamped local binary reports `dev`. The line is informational and does not alter
the existing header, URL, database, or log-level lines.
### Backend restart from the UI still rebuilds
The restart supervisor (ADR 0019) writes a launch manifest and listens on a control
@@ -250,3 +258,10 @@ them: building and serving the web app (Vite), the published npm shim, and repo
**THEN** it continues startup and prints the localhost URL without a `network:` line.
- **GIVEN** `KANDEV_SERVER_HOST` is a loopback or specific bind address, **WHEN** the
startup banner is printed, **THEN** it omits unrelated network addresses.
- **GIVEN** a stamped or unstamped launcher binary, **WHEN** `dev`, `start`, or `run`
prints its startup banner, **THEN** the banner includes `version:` with the same
value that `--version` prints, using `dev` for an unstamped binary.
## System design
The technical design for launcher startup identity is in [Go dev launcher and startup version](../system-design/go-dev-launcher.md).
@@ -0,0 +1,83 @@
---
status: draft
system: platform
requirements:
- REQ-PLATFORM-AGENT-PROCESS-EXIT-DRAIN-001
created: 2026-08-26
owners:
- kandev
---
# Agent process exit and stderr drain System Design
## Purpose and boundaries
The agentctl process manager owns the child process, an explicitly managed
stderr pipe, exit status, and safe recent-stderr diagnostics. This design
changes the stderr pipe ownership and the order in which process exit and
stderr completion are observed. It does not change agent protocol handling,
stderr sanitization, or process-group cleanup.
## Requirement mapping
| Requirement | Design section |
| --- | --- |
| `REQ-PLATFORM-AGENT-PROCESS-EXIT-DRAIN-001` | [Exit flow](#exit-flow) and [Failure and recovery](#failure-and-recovery) |
## Components and responsibilities
- `internal/agentctl/server/process.Manager` starts the managed command and
owns its lifecycle.
- `Manager.startProcessPipes` creates an `os.Pipe`, assigns its writer to the
command, and retains the reader and writer for explicit lifecycle cleanup.
- `Manager.readStderr` drains and sanitizes the current process generation's
stderr stream, appending only the safe projection to the ring buffer.
- `Manager.waitForExit` waits for the child process and then coordinates the
generation-specific `stderrDone` channel before publishing exit diagnostics.
- The adapter update channel receives the existing exit error event and recent
stderr fields.
## Data and contracts
The process manager keeps `processStderrDrainTimeout` as the upper bound for a
reader that does not finish after process exit. `stderrDone` remains local to a
process generation so a delayed reader from an old command cannot close a
replacement command's completion channel. The command receives the owned
stderr writer as an `*os.File`, so `Cmd.Wait` can reap the child without
closing the manager's reader before the reader drains. The parent writer is
closed immediately after a successful start, and the reader is closed after a
normal drain or when the bounded drain expires.
No HTTP, WebSocket, database, or agent protocol payload changes are required.
## Exit flow
1. Process startup creates the stdin and stdout pipes plus an explicitly owned
stderr `os.Pipe`, closes the parent stderr writer after the command starts,
and starts `readStderr` and `waitForExit` concurrently.
2. `readStderr` continues draining while the child is alive, so a child that
runs longer than the drain timeout does not create a timeout warning merely
because it has not exited.
3. `waitForExit` calls `cmd.Wait()` to observe the child exit.
4. After `cmd.Wait()` returns, `waitForExit` waits on the same generation's
`stderrDone` channel for the bounded drain interval. Because the stderr
reader is not a `Cmd.StderrPipe`, `Cmd.Wait` does not close it first.
5. Exit classification, recent-stderr event construction, process-group reap,
and final stopped status follow the existing paths.
## Failure and recovery
If the reader finishes after process exit, final exit diagnostics include the
same sanitized recent stderr. If the reader remains blocked, the manager emits
`timed out waiting for agent stderr to drain`, closes its owned reader to
unblock the goroutine, and continues within the existing bound instead of
hanging process cleanup. That warning then indicates a real post-exit pipe-
drain delay.
An intentional stop still uses the existing intentional-stop classification.
Non-zero process exits still retain their existing error event and exit code.
## Observability
The stderr-drain warning remains at warning level, but its occurrence is now
limited to the post-exit bounded wait. Existing process start, exit, and recent
stderr log fields remain unchanged.
@@ -0,0 +1,77 @@
---
status: draft
system: platform
requirements:
- REQ-PLATFORM-AGENTCTL-INSTANCE-STOP-001
created: 2026-08-26
owners:
- kandev
---
# Agentctl instance stop idempotency System Design
## Purpose and boundaries
The platform agentctl instance manager owns the lifecycle of each per-agent
HTTP server, process manager, and allocated port. The control server exposes
that lifecycle through `DELETE /api/v1/instances/:id`. This design makes a
completed duplicate stop benign while preserving failure and retry semantics.
## Requirement mapping
| Requirement | Design section |
| --- | --- |
| `REQ-PLATFORM-AGENTCTL-INSTANCE-STOP-001` | [Stop flow](#stop-flow) and [Failure and recovery](#failure-and-recovery) |
## Components and responsibilities
- `internal/agentctl/server/api.ControlServer` validates the requested ID and
maps the manager result to the existing HTTP response.
- `internal/agentctl/server/instance.Manager` snapshots the tracked instance,
serializes teardown with the instance's `stopMu`, and owns map and port
mutation.
- `instance.Instance` owns the per-instance stop lock and one-time port-release
state.
- `internal/agent/runtime/agentctl.ControlClient` retains its existing rule that
a 404 after a lost delete response satisfies the stopped postcondition.
## Data and contracts
The existing DELETE route and response bodies remain in use. An unknown ID is
still reported as HTTP 404. A stop that starts with a tracked instance and then
finds that the same instance has already been removed may complete as success;
an already-completed request that reaches the initial lookup after removal may
continue to receive HTTP 404. Neither completed-stop path is an HTTP 500.
If the ID is reused for a different tracked instance while an old stop finishes,
the manager must not treat that different pointer as the old completed stop.
That safety check prevents one lifecycle from mutating another lifecycle.
## Stop flow
1. The control handler performs its existing lookup and returns 404 for an ID
that is absent before the stop begins.
2. `Manager.StopInstance` snapshots the instance pointer and serializes against
other stops for that pointer with `stopMu`.
3. After acquiring the lock, the manager rechecks the ID mapping. If the mapping
is absent because this same pointer was already torn down and removed, it
returns the already-stopped success outcome. If a different pointer occupies
the ID, it retains the safety error.
4. The first successful teardown closes admission, stops the HTTP server and
process manager, releases the port once, and removes the pointer from the
instance map.
5. The control handler returns the existing success response for a nil manager
result. Real manager errors retain the existing failure response.
## Failure and recovery
Unknown instances remain non-mutating 404 responses. Completed duplicate stops
do not log an error or return 500. If HTTP-server or process-manager cleanup
fails, the manager retains the instance and allocated port for retry, and the
control handler keeps the error-level diagnostic and HTTP 500 response.
## Observability
The normal completed-stop path continues to log successful instance removal.
The already-stopped race path may use debug-level diagnostic context, but it
must not emit the current `failed to stop instance` error with a stack trace.
Real teardown failures keep their current error-level log and request status.
@@ -0,0 +1,77 @@
---
status: draft
system: platform
requirements:
- REQ-PLATFORM-GO-DEV-LAUNCHER-001
created: 2026-08-26
owners:
- kandev
---
# Go dev launcher and startup version System Design
## Purpose and boundaries
The platform launcher owns the startup banner shown by `dev`, `start`, and
`run`. This design adds build identity to the existing banner without changing
the backend health contract, service installation metadata, or network binding
behavior documented by the platform launcher requirement.
The launcher consumes build metadata from the native binary entrypoint. It does
not resolve a version from the database, the selected bundle directory, or an
environment variable used only for release display metadata.
## Requirement mapping
| Requirement | Design section |
| --- | --- |
| `REQ-PLATFORM-GO-DEV-LAUNCHER-001` | [Build identity](#build-identity) and [Startup flow](#startup-flow) |
## Components and responsibilities
- `apps/backend/cmd/kandev` owns the build-time `Version` variable and passes it
into `launcher.BuildInfo`.
- `internal/launcher.Run` selects the command and forwards the build version to
the selected launch path.
- `internal/launcher.runStart` and `internal/launcher.runInstalled` carry the
version in `managedAppConfig`.
- `internal/launcher.runDev` carries the version through the dev launch path.
- `internal/launcher.logStartup` renders the shared stdout banner.
## Build identity
`launcher.BuildInfo.Version` is the canonical value for the launcher banner. It
is the same value used by the native `--version` path and by the backend build
metadata when the binary is dispatched with `__backend`. The compiled default
is `dev`, so a local or otherwise unstamped binary never prints an empty version.
The existing `KANDEV_VERSION` value remains optional release display metadata for
the installed-release header. It does not override the binary build identity
shown on the `version:` line.
## Startup flow
Each shared launch mode resolves ports, endpoints, database path, and log level
before it calls the banner. The banner prints the existing header first, then a
single line in this shape:
```text
[kandev] version: <build version>
```
The line appears before the URL, network, MCP, database, and log-level lines.
Those existing lines retain their current order and values. The version line is
printed before child startup so it remains available when backend readiness
fails.
## Failure and recovery
If build metadata is empty at an internal call boundary, the launcher uses the
same `dev` fallback as the native binary defaults. Version rendering cannot
prevent launch, change endpoint selection, or change backend readiness behavior.
## Observability
The startup banner exposes the build identity on launcher stdout. `--version`
remains the machine-readable single-value command. The backend `/health` and
`/api/v1/system/info` version contracts remain independent but use the same
build metadata for the dispatched backend process.