fix: inherit service temporary environment (#1894)
This commit is contained in:
@@ -7,7 +7,6 @@ import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -73,26 +72,6 @@ func TestHandleVscodeStart_ManagerStopping(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleVscodeStart_TempSetupFailure(t *testing.T) {
|
||||
tempFile := filepath.Join(t.TempDir(), "not-a-directory")
|
||||
if err := os.WriteFile(tempFile, []byte("file"), 0o600); err != nil {
|
||||
t.Fatalf("write temp file: %v", err)
|
||||
}
|
||||
t.Setenv("TMPDIR", tempFile)
|
||||
s := newTestServer(t)
|
||||
|
||||
body, _ := json.Marshal(VscodeStartRequest{Theme: "dark"})
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/vscode/start", bytes.NewReader(body))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
w := httptest.NewRecorder()
|
||||
|
||||
s.router.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusInternalServerError {
|
||||
t.Errorf("expected 500, got %d", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleVscodeStop_WhenNotRunning(t *testing.T) {
|
||||
s := newTestServer(t)
|
||||
|
||||
|
||||
@@ -366,7 +366,7 @@ func (m *Manager) StopInstance(ctx context.Context, id string) error {
|
||||
}
|
||||
|
||||
stopErr := errors.Join(httpStopErr, processStopErr)
|
||||
if httpStopErr != nil || (processStopErr != nil && !canReleaseInstanceResources(processStopErr)) {
|
||||
if httpStopErr != nil || processStopErr != nil {
|
||||
return stopErr
|
||||
}
|
||||
|
||||
@@ -390,15 +390,6 @@ func (m *Manager) StopInstance(ctx context.Context, id string) error {
|
||||
return stopErr
|
||||
}
|
||||
|
||||
type instanceResourceReleaseError interface {
|
||||
CanReleaseInstanceResources() bool
|
||||
}
|
||||
|
||||
func canReleaseInstanceResources(err error) bool {
|
||||
var classified instanceResourceReleaseError
|
||||
return errors.As(err, &classified) && classified.CanReleaseInstanceResources()
|
||||
}
|
||||
|
||||
type instanceHTTPServer interface {
|
||||
Shutdown(context.Context) error
|
||||
Close() error
|
||||
|
||||
@@ -153,7 +153,7 @@ func TestStopInstanceRetainsPortWhenHTTPServerCloseFails(t *testing.T) {
|
||||
shutdownErr: context.DeadlineExceeded,
|
||||
closeErr: closeErr,
|
||||
},
|
||||
manager: &fakeProcessManager{stopErr: cleanupOnlyError{err: errors.New("temp cleanup failed")}},
|
||||
manager: &fakeProcessManager{stopErr: errors.New("process teardown failed")},
|
||||
}
|
||||
|
||||
mgr.mu.Lock()
|
||||
@@ -167,7 +167,7 @@ func TestStopInstanceRetainsPortWhenHTTPServerCloseFails(t *testing.T) {
|
||||
require.Error(t, err)
|
||||
}
|
||||
|
||||
func TestStopInstanceReleasesPortAfterTempCleanupFailure(t *testing.T) {
|
||||
func TestStopInstanceRetainsPortAfterProcessTeardownFailure(t *testing.T) {
|
||||
log := newTestLogger(t)
|
||||
mgr := NewManager(&config.Config{
|
||||
Ports: config.PortConfig{Base: 12345, Max: 12345},
|
||||
@@ -176,8 +176,8 @@ func TestStopInstanceReleasesPortAfterTempCleanupFailure(t *testing.T) {
|
||||
|
||||
port, err := mgr.portAlloc.Allocate("process-cleanup-failure")
|
||||
require.NoError(t, err)
|
||||
processErr := errors.New("agent temp cleanup failed")
|
||||
procMgr := &fakeProcessManager{stopErr: cleanupOnlyError{err: processErr}}
|
||||
processErr := errors.New("process teardown failed")
|
||||
procMgr := &fakeProcessManager{stopErr: legacyResourceReleaseError{err: processErr}}
|
||||
server := &fakeHTTPServer{}
|
||||
inst := &Instance{
|
||||
ID: "process-cleanup-failure",
|
||||
@@ -196,17 +196,17 @@ func TestStopInstanceReleasesPortAfterTempCleanupFailure(t *testing.T) {
|
||||
require.True(t, procMgr.stopped)
|
||||
require.True(t, server.shutdown)
|
||||
_, ok := mgr.GetInstance(inst.ID)
|
||||
require.True(t, ok, "cleanup-only failure must retain a retry tombstone")
|
||||
reusedPort, err := mgr.portAlloc.Allocate("next-instance")
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, port, reusedPort)
|
||||
require.True(t, ok, "process teardown failure must retain the instance for retry")
|
||||
_, err = mgr.portAlloc.Allocate("next-instance")
|
||||
require.Error(t, err, "process teardown failure must retain the instance port")
|
||||
|
||||
procMgr.stopErr = nil
|
||||
require.NoError(t, mgr.StopInstance(context.Background(), inst.ID))
|
||||
_, ok = mgr.GetInstance(inst.ID)
|
||||
require.False(t, ok)
|
||||
_, err = mgr.portAlloc.Allocate("third-instance")
|
||||
require.Error(t, err, "retry must not release a port that was reassigned")
|
||||
reusedPort, err := mgr.portAlloc.Allocate("third-instance")
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, port, reusedPort)
|
||||
}
|
||||
|
||||
func TestStopHTTPServerTreatsCanceledShutdownAsStoppedAfterClose(t *testing.T) {
|
||||
@@ -245,13 +245,15 @@ type fakeProcessManager struct {
|
||||
stopped bool
|
||||
}
|
||||
|
||||
type cleanupOnlyError struct {
|
||||
// legacyResourceReleaseError proves a process teardown error cannot authorize
|
||||
// releasing the instance's port while its process ownership is unresolved.
|
||||
type legacyResourceReleaseError struct {
|
||||
err error
|
||||
}
|
||||
|
||||
func (e cleanupOnlyError) Error() string { return e.err.Error() }
|
||||
func (e cleanupOnlyError) Unwrap() error { return e.err }
|
||||
func (e cleanupOnlyError) CanReleaseInstanceResources() bool {
|
||||
func (e legacyResourceReleaseError) Error() string { return e.err.Error() }
|
||||
func (e legacyResourceReleaseError) Unwrap() error { return e.err }
|
||||
func (e legacyResourceReleaseError) CanReleaseInstanceResources() bool {
|
||||
return true
|
||||
}
|
||||
|
||||
|
||||
@@ -4,7 +4,6 @@ package process
|
||||
import (
|
||||
"bufio"
|
||||
"context"
|
||||
"crypto/sha256"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
@@ -70,8 +69,6 @@ type PermissionNotification struct {
|
||||
// defaultStderrBufferSize is the number of recent stderr lines to keep for error context
|
||||
const defaultStderrBufferSize = 50
|
||||
|
||||
const agentTempDirRoot = "kandev-agent"
|
||||
|
||||
const processKillRequiredExitGrace = 250 * time.Millisecond
|
||||
|
||||
// processDefaultExitGrace is the no-deadline upper bound for graceful adapters
|
||||
@@ -183,15 +180,6 @@ type Manager struct {
|
||||
// Final command string (full command with all adapter args)
|
||||
finalCommand string
|
||||
|
||||
// agentTempRoot and agentTempDir record the exact temporary directory
|
||||
// created for this manager. The retained Root keeps cleanup anchored to the
|
||||
// directory opened at creation even if its pathname is later replaced.
|
||||
agentTempRoot string
|
||||
agentTempDir string
|
||||
agentTempChild string
|
||||
agentTempRootHandle *os.Root
|
||||
agentTempMu sync.Mutex
|
||||
|
||||
// Synchronization
|
||||
mu sync.RWMutex
|
||||
wg sync.WaitGroup
|
||||
@@ -602,10 +590,6 @@ func (m *Manager) StartProcess(ctx context.Context, req StartProcessRequest) (*P
|
||||
if m.processRunner == nil {
|
||||
return nil, fmt.Errorf("process runner not available")
|
||||
}
|
||||
if err := m.ensureAgentTempEnv(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
req.Env = m.ownedProcessEnv(req.Env)
|
||||
return m.processRunner.Start(ctx, req)
|
||||
}
|
||||
|
||||
@@ -819,11 +803,6 @@ func (m *Manager) Start(ctx context.Context) error {
|
||||
return fmt.Errorf("no agent command configured")
|
||||
}
|
||||
|
||||
if err := m.ensureAgentTempEnv(); err != nil {
|
||||
m.status.Store(StatusError)
|
||||
return err
|
||||
}
|
||||
|
||||
// Build adapter config and create protocol adapter
|
||||
if err := m.buildAdapterConfig(); err != nil {
|
||||
m.status.Store(StatusError)
|
||||
@@ -908,11 +887,6 @@ func (m *Manager) Start(ctx context.Context) error {
|
||||
// startOneShot initialises a one-shot adapter without spawning a long-lived subprocess.
|
||||
// The adapter manages its own per-prompt subprocess lifecycle internally.
|
||||
func (m *Manager) startOneShot() error {
|
||||
// One-shot adapters bypass buildFinalCommand, so restore temp env vars
|
||||
// after StripEnv before their per-prompt subprocesses inherit Env.
|
||||
if err := m.ensureAgentTempEnv(); err != nil {
|
||||
return err
|
||||
}
|
||||
if m.adapterCfg != nil && m.adapterCfg.OneShotConfig != nil {
|
||||
m.adapterCfg.OneShotConfig.Env = m.cfg.AgentEnv
|
||||
}
|
||||
@@ -1014,10 +988,6 @@ func (m *Manager) buildAdapterConfig() error {
|
||||
// buildFinalCommand assembles the full command args and creates the exec.Cmd.
|
||||
// The process group is set so child processes can be killed together.
|
||||
func (m *Manager) buildFinalCommand() error {
|
||||
if err := m.ensureAgentTempEnv(); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
extraArgs := m.adapter.PrepareCommandArgs()
|
||||
|
||||
cmdArgs := make([]string, 0, len(m.cfg.AgentArgs)-1+len(extraArgs))
|
||||
@@ -1111,167 +1081,6 @@ func formatEnvEntrySizes(entries []envEntrySize) string {
|
||||
return "[" + strings.Join(parts, ",") + "]"
|
||||
}
|
||||
|
||||
func (m *Manager) ensureAgentTempEnv() error {
|
||||
m.agentTempMu.Lock()
|
||||
defer m.agentTempMu.Unlock()
|
||||
|
||||
root := filepath.Join(os.TempDir(), agentTempDirRoot)
|
||||
child := agentTempDirName(m.cfg.SessionID, m.cfg.InstanceID, m.cfg.Port)
|
||||
dir := filepath.Join(root, child)
|
||||
if m.agentTempRootHandle != nil {
|
||||
if m.agentTempRoot == root && m.agentTempDir == dir && m.agentTempChild == child {
|
||||
for _, key := range []string{"TMPDIR", "TMP", "TEMP"} {
|
||||
m.cfg.AgentEnv = upsertEnvValue(m.cfg.AgentEnv, key, dir)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
return fmt.Errorf("agent temp ownership already initialized for %q", m.agentTempDir)
|
||||
}
|
||||
rootHandle, err := openOwnedTempRoot(root)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if err := ensureOwnedTempChild(rootHandle, child); err != nil {
|
||||
_ = rootHandle.Close()
|
||||
return fmt.Errorf("failed to create agent temp dir: %w", err)
|
||||
}
|
||||
m.agentTempRoot = root
|
||||
m.agentTempDir = dir
|
||||
m.agentTempChild = child
|
||||
m.agentTempRootHandle = rootHandle
|
||||
for _, key := range []string{"TMPDIR", "TMP", "TEMP"} {
|
||||
m.cfg.AgentEnv = upsertEnvValue(m.cfg.AgentEnv, key, dir)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *Manager) ownedProcessEnv(env map[string]string) map[string]string {
|
||||
m.agentTempMu.Lock()
|
||||
defer m.agentTempMu.Unlock()
|
||||
|
||||
managed := make(map[string]string, len(env)+3)
|
||||
for key, value := range env {
|
||||
managed[key] = value
|
||||
}
|
||||
for _, key := range []string{"TMPDIR", "TMP", "TEMP"} {
|
||||
managed[key] = m.agentTempDir
|
||||
}
|
||||
return managed
|
||||
}
|
||||
|
||||
func openOwnedTempRoot(path string) (*os.Root, error) {
|
||||
if err := os.Mkdir(path, 0o700); err != nil && !errors.Is(err, os.ErrExist) {
|
||||
return nil, fmt.Errorf("failed to create agent temp root %q: %w", path, err)
|
||||
}
|
||||
pathInfo, err := os.Lstat(path)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to inspect agent temp root %q: %w", path, err)
|
||||
}
|
||||
if pathInfo.Mode()&os.ModeSymlink != 0 || !pathInfo.IsDir() {
|
||||
return nil, fmt.Errorf("unsafe agent temp root %q: expected a directory, not a symlink", path)
|
||||
}
|
||||
root, err := os.OpenRoot(path)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to open agent temp root %q: %w", path, err)
|
||||
}
|
||||
openedInfo, err := root.Stat(".")
|
||||
if err != nil || !os.SameFile(pathInfo, openedInfo) {
|
||||
_ = root.Close()
|
||||
return nil, fmt.Errorf("unsafe agent temp root %q: path changed while opening", path)
|
||||
}
|
||||
if err := root.Chmod(".", 0o700); err != nil {
|
||||
_ = root.Close()
|
||||
return nil, fmt.Errorf("failed to secure agent temp root %q: %w", path, err)
|
||||
}
|
||||
return root, nil
|
||||
}
|
||||
|
||||
func ensureOwnedTempChild(root *os.Root, name string) error {
|
||||
if err := root.Mkdir(name, 0o700); err != nil && !errors.Is(err, os.ErrExist) {
|
||||
return err
|
||||
}
|
||||
info, err := root.Lstat(name)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if info.Mode()&os.ModeSymlink != 0 || !info.IsDir() {
|
||||
return fmt.Errorf("unsafe agent temp child %q: expected a directory, not a symlink", name)
|
||||
}
|
||||
return root.Chmod(name, 0o700)
|
||||
}
|
||||
|
||||
func (m *Manager) cleanupAgentTempDir() error {
|
||||
m.agentTempMu.Lock()
|
||||
defer m.agentTempMu.Unlock()
|
||||
|
||||
root := filepath.Clean(m.agentTempRoot)
|
||||
target := filepath.Clean(m.agentTempDir)
|
||||
if m.agentTempRoot == "" && m.agentTempDir == "" {
|
||||
return nil
|
||||
}
|
||||
if m.agentTempRoot == "" || m.agentTempDir == "" || target == root {
|
||||
return fmt.Errorf("refusing to clean invalid agent temp dir %q under root %q", m.agentTempDir, m.agentTempRoot)
|
||||
}
|
||||
rel, err := filepath.Rel(root, target)
|
||||
if err != nil || filepath.IsAbs(rel) || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
|
||||
return fmt.Errorf("refusing to clean agent temp dir outside root: target %q root %q", m.agentTempDir, m.agentTempRoot)
|
||||
}
|
||||
if rel != m.agentTempChild || strings.ContainsRune(rel, filepath.Separator) || m.agentTempRootHandle == nil {
|
||||
return fmt.Errorf("refusing to clean unowned agent temp dir %q under root %q", m.agentTempDir, m.agentTempRoot)
|
||||
}
|
||||
if err := m.agentTempRootHandle.RemoveAll(rel); err != nil {
|
||||
return fmt.Errorf("failed to remove agent temp dir %q: %w", target, err)
|
||||
}
|
||||
closeErr := m.agentTempRootHandle.Close()
|
||||
m.agentTempRoot = ""
|
||||
m.agentTempDir = ""
|
||||
m.agentTempChild = ""
|
||||
m.agentTempRootHandle = nil
|
||||
if closeErr != nil {
|
||||
return fmt.Errorf("failed to close agent temp root %q: %w", root, closeErr)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func agentTempDirName(sessionID, instanceID string, port int) string {
|
||||
name := strings.TrimSpace(sessionID)
|
||||
if name == "" {
|
||||
name = strings.TrimSpace(instanceID)
|
||||
}
|
||||
if name == "" && port > 0 {
|
||||
name = fmt.Sprintf("port-%d", port)
|
||||
}
|
||||
if name == "" {
|
||||
name = "default"
|
||||
}
|
||||
|
||||
var b strings.Builder
|
||||
for _, r := range name {
|
||||
if isAgentTempDirRune(r) {
|
||||
b.WriteRune(r)
|
||||
} else {
|
||||
b.WriteByte('_')
|
||||
}
|
||||
}
|
||||
cleaned := strings.Trim(b.String(), "._-")
|
||||
if cleaned == "" {
|
||||
cleaned = "default"
|
||||
}
|
||||
if len(cleaned) > 40 {
|
||||
cleaned = cleaned[:40]
|
||||
}
|
||||
identity := fmt.Sprintf("%d:%s|%d:%s|%d", len(sessionID), sessionID, len(instanceID), instanceID, port)
|
||||
digest := sha256.Sum256([]byte(identity))
|
||||
return fmt.Sprintf("%s-%x", cleaned, digest[:12])
|
||||
}
|
||||
|
||||
func isAgentTempDirRune(r rune) bool {
|
||||
return r >= 'a' && r <= 'z' ||
|
||||
r >= 'A' && r <= 'Z' ||
|
||||
r >= '0' && r <= '9' ||
|
||||
r == '.' || r == '_' || r == '-'
|
||||
}
|
||||
|
||||
func upsertEnvValue(env []string, key, value string) []string {
|
||||
prefix := key + "="
|
||||
next := make([]string, 0, len(env)+1)
|
||||
@@ -1322,7 +1131,6 @@ func (m *Manager) startAgentShell() {
|
||||
}
|
||||
shellCfg := shell.DefaultConfig(m.cfg.WorkDir)
|
||||
shellCfg.ShellCommand = preferredShellCommand(m.cfg.AgentEnv)
|
||||
shellCfg.Env = m.ownedProcessEnv(nil)
|
||||
shellSession, err := shell.NewSession(shellCfg, m.logger)
|
||||
if err != nil {
|
||||
m.logger.Warn("failed to create shell session", zap.Error(err))
|
||||
@@ -1489,20 +1297,20 @@ func (m *Manager) GetSessionID() string {
|
||||
|
||||
// Stop stops the agent process
|
||||
func (m *Manager) Stop(ctx context.Context) error {
|
||||
return m.stop(ctx, false)
|
||||
return m.stop(ctx)
|
||||
}
|
||||
|
||||
// StopForTeardown permanently closes process admission, drains prior owners,
|
||||
// and removes the manager's temp directory after every owned process is reaped.
|
||||
// StopForTeardown permanently closes process admission and drains prior owners
|
||||
// before stopping every process owned by the manager.
|
||||
func (m *Manager) StopForTeardown(ctx context.Context) error {
|
||||
m.CloseAdmission()
|
||||
if err := m.WaitForAdmission(ctx); err != nil {
|
||||
return fmt.Errorf("wait for process admission to drain: %w", err)
|
||||
}
|
||||
return m.stop(ctx, true)
|
||||
return m.stop(ctx)
|
||||
}
|
||||
|
||||
func (m *Manager) stop(ctx context.Context, terminalStop bool) error {
|
||||
func (m *Manager) stop(ctx context.Context) error {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
|
||||
@@ -1529,9 +1337,6 @@ func (m *Manager) stop(ctx context.Context, terminalStop bool) error {
|
||||
}
|
||||
m.mainReapPending.Store(false)
|
||||
}
|
||||
if terminalStop {
|
||||
return classifyTempCleanupError(m.cleanupAgentTempDir())
|
||||
}
|
||||
return nil
|
||||
}
|
||||
return nil
|
||||
@@ -1557,29 +1362,9 @@ func (m *Manager) stop(ctx context.Context, terminalStop bool) error {
|
||||
return stopErr
|
||||
}
|
||||
m.mainReapPending.Store(false)
|
||||
if terminalStop {
|
||||
return classifyTempCleanupError(m.cleanupAgentTempDir())
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
type tempCleanupError struct {
|
||||
err error
|
||||
}
|
||||
|
||||
func (e tempCleanupError) Error() string { return e.err.Error() }
|
||||
func (e tempCleanupError) Unwrap() error { return e.err }
|
||||
func (e tempCleanupError) CanReleaseInstanceResources() bool {
|
||||
return true
|
||||
}
|
||||
|
||||
func classifyTempCleanupError(err error) error {
|
||||
if err == nil {
|
||||
return nil
|
||||
}
|
||||
return tempCleanupError{err: err}
|
||||
}
|
||||
|
||||
func (m *Manager) agentPID() int {
|
||||
if m.cmd == nil || m.cmd.Process == nil {
|
||||
return 0
|
||||
@@ -2338,13 +2123,8 @@ func (m *Manager) StartShell() error {
|
||||
if !m.cfg.ShellEnabled {
|
||||
return nil
|
||||
}
|
||||
if err := m.ensureAgentTempEnv(); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
shellCfg := shell.DefaultConfig(m.cfg.WorkDir)
|
||||
shellCfg.ShellCommand = preferredShellCommand(m.cfg.AgentEnv)
|
||||
shellCfg.Env = m.ownedProcessEnv(nil)
|
||||
shellSession, err := shell.NewSession(shellCfg, m.logger)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to create shell session: %w", err)
|
||||
@@ -2355,17 +2135,13 @@ func (m *Manager) StartShell() error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// StartTerminalShell creates a managed per-terminal shell with the instance temp environment.
|
||||
// StartTerminalShell creates a managed per-terminal shell.
|
||||
func (m *Manager) StartTerminalShell(terminalID string, cfg shell.Config) (*shell.Session, error) {
|
||||
release, err := m.admitStart()
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer release()
|
||||
if err := m.ensureAgentTempEnv(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
cfg.Env = m.ownedProcessEnv(cfg.Env)
|
||||
return m.shellMgr.Start(terminalID, cfg)
|
||||
}
|
||||
|
||||
@@ -2378,9 +2154,6 @@ func (m *Manager) StartVscode(_ context.Context, theme string) error {
|
||||
return err
|
||||
}
|
||||
defer release()
|
||||
if err := m.ensureAgentTempEnv(); err != nil {
|
||||
return err
|
||||
}
|
||||
return m.startVscode(theme)
|
||||
}
|
||||
|
||||
@@ -2405,7 +2178,6 @@ func (m *Manager) startVscode(theme string) error {
|
||||
|
||||
strategy := codeServerInstallStrategy(m.logger)
|
||||
m.vscode = NewVscodeManager(command, m.cfg.WorkDir, theme, strategy, m.logger)
|
||||
m.vscode.setEnv(m.ownedProcessEnv(nil))
|
||||
m.vscode.Start()
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -3,422 +3,90 @@ package process
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"io"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/kandev/kandev/internal/agentctl/server/adapter"
|
||||
"github.com/kandev/kandev/internal/agentctl/server/config"
|
||||
"github.com/kandev/kandev/internal/agentctl/server/shell"
|
||||
"github.com/kandev/kandev/internal/agentctl/types"
|
||||
v1 "github.com/kandev/kandev/pkg/api/v1"
|
||||
)
|
||||
|
||||
type stubAgentAdapter struct{}
|
||||
|
||||
type notifyingCloser struct {
|
||||
closed chan struct{}
|
||||
once sync.Once
|
||||
}
|
||||
|
||||
func (c *notifyingCloser) Write(p []byte) (int, error) { return len(p), nil }
|
||||
|
||||
func (c *notifyingCloser) Close() error {
|
||||
c.once.Do(func() { close(c.closed) })
|
||||
return nil
|
||||
}
|
||||
|
||||
func (stubAgentAdapter) PrepareEnvironment() (map[string]string, error) { return nil, nil }
|
||||
func (stubAgentAdapter) PrepareCommandArgs() []string { return nil }
|
||||
func (stubAgentAdapter) Connect(io.Writer, io.Reader) error { return nil }
|
||||
func (stubAgentAdapter) Initialize(context.Context) error { return nil }
|
||||
func (stubAgentAdapter) GetAgentInfo() *adapter.AgentInfo { return nil }
|
||||
func (stubAgentAdapter) NewSession(context.Context, []types.McpServer) (string, error) {
|
||||
return "", nil
|
||||
}
|
||||
func (stubAgentAdapter) LoadSession(context.Context, string, []types.McpServer) error {
|
||||
return nil
|
||||
}
|
||||
func (stubAgentAdapter) Prompt(context.Context, string, []v1.MessageAttachment, uint64) error {
|
||||
return nil
|
||||
}
|
||||
func (stubAgentAdapter) Cancel(context.Context) error { return nil }
|
||||
func (stubAgentAdapter) Updates() <-chan adapter.AgentEvent { return nil }
|
||||
func (stubAgentAdapter) GetSessionID() string { return "" }
|
||||
func (stubAgentAdapter) GetOperationID() string { return "" }
|
||||
func (stubAgentAdapter) SetPermissionHandler(adapter.PermissionHandler) {}
|
||||
func (stubAgentAdapter) Close() error { return nil }
|
||||
func (stubAgentAdapter) RequiresProcessKill() bool { return false }
|
||||
|
||||
func envValue(env []string, key string) string {
|
||||
prefix := key + "="
|
||||
for _, item := range env {
|
||||
if len(item) > len(prefix) && item[:len(prefix)] == prefix {
|
||||
return item[len(prefix):]
|
||||
if strings.HasPrefix(item, prefix) {
|
||||
return strings.TrimPrefix(item, prefix)
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
func TestManager_BuildFinalCommandInjectsIsolatedTempDir(t *testing.T) {
|
||||
setAgentTempTestEnv(t)
|
||||
func hasEnvValue(env []string, key string) bool {
|
||||
prefix := key + "="
|
||||
for _, item := range env {
|
||||
if strings.HasPrefix(item, prefix) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func TestManager_BuildFinalCommandPreservesConfiguredTempEnvironment(t *testing.T) {
|
||||
serviceTemp := setServiceTempTestEnv(t)
|
||||
mgr := NewManager(&config.InstanceConfig{
|
||||
WorkDir: t.TempDir(),
|
||||
SessionID: "session-123",
|
||||
AgentArgs: []string{"echo"},
|
||||
AgentEnv: []string{"PATH=/usr/bin"},
|
||||
AgentEnv: []string{
|
||||
"PATH=/usr/bin",
|
||||
"TMPDIR=/configured/tmpdir",
|
||||
"TMP=/configured/tmp",
|
||||
"TEMP=/configured/temp",
|
||||
},
|
||||
}, newTestLogger(t))
|
||||
mgr.adapter = stubAgentAdapter{}
|
||||
t.Cleanup(func() { _ = mgr.StopForTeardown(context.Background()) })
|
||||
mgr.adapter = newStubAdapter()
|
||||
|
||||
if err := mgr.buildFinalCommand(); err != nil {
|
||||
t.Fatalf("buildFinalCommand() error = %v", err)
|
||||
}
|
||||
|
||||
want := map[string]string{
|
||||
"TMPDIR": "/configured/tmpdir",
|
||||
"TMP": "/configured/tmp",
|
||||
"TEMP": "/configured/temp",
|
||||
}
|
||||
for key, value := range want {
|
||||
if got := envValue(mgr.cmd.Env, key); got != value {
|
||||
t.Fatalf("%s = %q, want configured service value %q", key, got, value)
|
||||
}
|
||||
}
|
||||
assertNoAgentTempRoot(t, serviceTemp)
|
||||
}
|
||||
|
||||
func TestManager_BuildFinalCommandLeavesUnsetTempEnvironmentUnset(t *testing.T) {
|
||||
serviceTemp := setServiceTempTestEnv(t)
|
||||
mgr := NewManager(&config.InstanceConfig{
|
||||
WorkDir: t.TempDir(),
|
||||
AgentArgs: []string{"echo"},
|
||||
AgentEnv: []string{"PATH=/usr/bin"},
|
||||
}, newTestLogger(t))
|
||||
mgr.adapter = newStubAdapter()
|
||||
|
||||
if err := mgr.buildFinalCommand(); err != nil {
|
||||
t.Fatalf("buildFinalCommand() error = %v", err)
|
||||
}
|
||||
|
||||
want := filepath.Join(os.TempDir(), "kandev-agent", agentTempDirName("session-123", "", 0))
|
||||
for _, key := range []string{"TMPDIR", "TMP", "TEMP"} {
|
||||
if got := envValue(mgr.cmd.Env, key); got != want {
|
||||
t.Fatalf("%s = %q, want %q", key, got, want)
|
||||
if hasEnvValue(mgr.cmd.Env, key) {
|
||||
t.Fatalf("%s unexpectedly added to child environment: %q", key, mgr.cmd.Env)
|
||||
}
|
||||
}
|
||||
if info, err := os.Stat(want); err != nil || !info.IsDir() {
|
||||
t.Fatalf("expected temp dir %q to exist, stat=%v err=%v", want, info, err)
|
||||
}
|
||||
assertNoAgentTempRoot(t, serviceTemp)
|
||||
}
|
||||
|
||||
func TestManager_OwnedProcessEnvOverridesRequestedTemp(t *testing.T) {
|
||||
setAgentTempTestEnv(t)
|
||||
mgr := NewManager(&config.InstanceConfig{
|
||||
WorkDir: t.TempDir(),
|
||||
SessionID: "session-child-env",
|
||||
}, newTestLogger(t))
|
||||
if err := mgr.ensureAgentTempEnv(); err != nil {
|
||||
t.Fatalf("ensureAgentTempEnv() error = %v", err)
|
||||
}
|
||||
|
||||
env := mgr.ownedProcessEnv(map[string]string{
|
||||
"TMPDIR": "unmanaged",
|
||||
"CUSTOM": "preserved",
|
||||
})
|
||||
for _, key := range []string{"TMPDIR", "TMP", "TEMP"} {
|
||||
if got := env[key]; got != mgr.agentTempDir {
|
||||
t.Fatalf("%s = %q, want %q", key, got, mgr.agentTempDir)
|
||||
}
|
||||
}
|
||||
if got := env["CUSTOM"]; got != "preserved" {
|
||||
t.Fatalf("CUSTOM = %q, want preserved", got)
|
||||
}
|
||||
if err := mgr.StopForTeardown(context.Background()); err != nil {
|
||||
t.Fatalf("StopForTeardown() error = %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestManager_StopRemovesAlreadyStoppedTempDirAndPreservesSibling(t *testing.T) {
|
||||
setAgentTempTestEnv(t)
|
||||
mgr := NewManager(&config.InstanceConfig{
|
||||
WorkDir: t.TempDir(),
|
||||
SessionID: "session-owned",
|
||||
}, newTestLogger(t))
|
||||
|
||||
if err := mgr.ensureAgentTempEnv(); err != nil {
|
||||
t.Fatalf("ensureAgentTempEnv() error = %v", err)
|
||||
}
|
||||
owned := envValue(mgr.cfg.AgentEnv, "TMPDIR")
|
||||
sibling := filepath.Join(filepath.Dir(owned), "session-sibling")
|
||||
if err := os.MkdirAll(sibling, 0o700); err != nil {
|
||||
t.Fatalf("create sibling temp dir: %v", err)
|
||||
}
|
||||
|
||||
if err := mgr.StopForTeardown(context.Background()); err != nil {
|
||||
t.Fatalf("Stop() error = %v", err)
|
||||
}
|
||||
if _, err := os.Stat(owned); !os.IsNotExist(err) {
|
||||
t.Fatalf("owned temp dir still exists after Stop(), err = %v", err)
|
||||
}
|
||||
if info, err := os.Stat(sibling); err != nil || !info.IsDir() {
|
||||
t.Fatalf("sibling temp dir was changed, stat = %v, err = %v", info, err)
|
||||
}
|
||||
if info, err := os.Stat(filepath.Dir(owned)); err != nil || !info.IsDir() {
|
||||
t.Fatalf("shared temp root was changed, stat = %v, err = %v", info, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestManager_StopRemovesTempOnlyAfterProcessTeardown(t *testing.T) {
|
||||
setAgentTempTestEnv(t)
|
||||
mgr := NewManager(&config.InstanceConfig{
|
||||
WorkDir: t.TempDir(),
|
||||
SessionID: "session-running",
|
||||
}, newTestLogger(t))
|
||||
if err := mgr.ensureAgentTempEnv(); err != nil {
|
||||
t.Fatalf("ensureAgentTempEnv() error = %v", err)
|
||||
}
|
||||
owned := envValue(mgr.cfg.AgentEnv, "TMPDIR")
|
||||
|
||||
stdin := ¬ifyingCloser{closed: make(chan struct{})}
|
||||
mgr.stdin = stdin
|
||||
mgr.status.Store(StatusRunning)
|
||||
reap := make(chan struct{})
|
||||
var releaseOnce sync.Once
|
||||
release := func() { releaseOnce.Do(func() { close(reap) }) }
|
||||
mgr.wg.Add(1)
|
||||
go func() {
|
||||
defer mgr.wg.Done()
|
||||
<-reap
|
||||
}()
|
||||
t.Cleanup(release)
|
||||
|
||||
stopDone := make(chan error, 1)
|
||||
go func() { stopDone <- mgr.StopForTeardown(context.Background()) }()
|
||||
<-stdin.closed
|
||||
if info, err := os.Stat(owned); err != nil || !info.IsDir() {
|
||||
t.Fatalf("temp dir removed before process reap, stat = %v, err = %v", info, err)
|
||||
}
|
||||
|
||||
release()
|
||||
if err := <-stopDone; err != nil {
|
||||
t.Fatalf("Stop() error = %v", err)
|
||||
}
|
||||
if _, err := os.Stat(owned); !os.IsNotExist(err) {
|
||||
t.Fatalf("owned temp dir still exists after process reap, err = %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestManager_StopUsesStoredTempOwnershipInsteadOfMutableEnvironment(t *testing.T) {
|
||||
setAgentTempTestEnv(t)
|
||||
mgr := NewManager(&config.InstanceConfig{
|
||||
WorkDir: t.TempDir(),
|
||||
SessionID: "session-stored",
|
||||
}, newTestLogger(t))
|
||||
if err := mgr.ensureAgentTempEnv(); err != nil {
|
||||
t.Fatalf("ensureAgentTempEnv() error = %v", err)
|
||||
}
|
||||
owned := envValue(mgr.cfg.AgentEnv, "TMPDIR")
|
||||
outside := filepath.Join(t.TempDir(), "must-remain")
|
||||
if err := os.MkdirAll(outside, 0o700); err != nil {
|
||||
t.Fatalf("create outside dir: %v", err)
|
||||
}
|
||||
mgr.cfg.AgentEnv = upsertEnvValue(mgr.cfg.AgentEnv, "TMPDIR", outside)
|
||||
|
||||
if err := mgr.StopForTeardown(context.Background()); err != nil {
|
||||
t.Fatalf("Stop() error = %v", err)
|
||||
}
|
||||
if _, err := os.Stat(owned); !os.IsNotExist(err) {
|
||||
t.Fatalf("stored owned temp dir still exists, err = %v", err)
|
||||
}
|
||||
if info, err := os.Stat(outside); err != nil || !info.IsDir() {
|
||||
t.Fatalf("environment-selected outside dir was changed, stat = %v, err = %v", info, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestManager_StopRejectsInvalidTempCleanupTargets(t *testing.T) {
|
||||
base := t.TempDir()
|
||||
root := filepath.Join(base, agentTempDirRoot)
|
||||
outside := filepath.Join(base, "outside")
|
||||
for _, path := range []string{root, outside} {
|
||||
if err := os.MkdirAll(path, 0o700); err != nil {
|
||||
t.Fatalf("create fixture dir %q: %v", path, err)
|
||||
}
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
root string
|
||||
target string
|
||||
}{
|
||||
{name: "empty target", root: root},
|
||||
{name: "shared root", root: root, target: root},
|
||||
{name: "outside root", root: root, target: outside},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
mgr := NewManager(&config.InstanceConfig{WorkDir: t.TempDir()}, newTestLogger(t))
|
||||
mgr.agentTempRoot = tt.root
|
||||
mgr.agentTempDir = tt.target
|
||||
|
||||
err := mgr.StopForTeardown(context.Background())
|
||||
if err == nil || !strings.Contains(err.Error(), "refusing to clean") {
|
||||
t.Fatalf("Stop() error = %v, want fail-closed cleanup error", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
for _, path := range []string{root, outside} {
|
||||
if info, err := os.Stat(path); err != nil || !info.IsDir() {
|
||||
t.Fatalf("invalid cleanup changed %q, stat = %v, err = %v", path, info, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestManager_StopRejectsTempCleanupWithoutOwnershipHandle(t *testing.T) {
|
||||
base := t.TempDir()
|
||||
root := filepath.Join(base, agentTempDirRoot)
|
||||
if err := os.Mkdir(root, 0o700); err != nil {
|
||||
t.Fatalf("create root dir: %v", err)
|
||||
}
|
||||
mgr := NewManager(&config.InstanceConfig{WorkDir: t.TempDir()}, newTestLogger(t))
|
||||
mgr.agentTempRoot = root
|
||||
mgr.agentTempDir = filepath.Join(root, "session\x00invalid")
|
||||
|
||||
err := mgr.StopForTeardown(context.Background())
|
||||
if err == nil || !strings.Contains(err.Error(), "refusing to clean unowned") {
|
||||
t.Fatalf("Stop() error = %v, want fail-closed ownership error", err)
|
||||
}
|
||||
if mgr.agentTempDir == "" {
|
||||
t.Fatal("cleanup ownership cleared after failed removal")
|
||||
}
|
||||
}
|
||||
|
||||
func TestManager_EnsureTempRejectsSymlinkedSharedRoot(t *testing.T) {
|
||||
tempBase := setAgentTempTestEnv(t)
|
||||
outside := t.TempDir()
|
||||
root := filepath.Join(tempBase, agentTempDirRoot)
|
||||
if err := os.Symlink(outside, root); err != nil {
|
||||
t.Skipf("create symlink fixture: %v", err)
|
||||
}
|
||||
mgr := NewManager(&config.InstanceConfig{
|
||||
WorkDir: t.TempDir(),
|
||||
SessionID: "session-symlink",
|
||||
}, newTestLogger(t))
|
||||
|
||||
err := mgr.ensureAgentTempEnv()
|
||||
if err == nil || !strings.Contains(err.Error(), "agent temp root") {
|
||||
t.Fatalf("ensureAgentTempEnv() error = %v, want unsafe-root error", err)
|
||||
}
|
||||
if _, err := os.Stat(filepath.Join(outside, "session-symlink")); !os.IsNotExist(err) {
|
||||
t.Fatalf("ensureAgentTempEnv() traversed symlinked root, err = %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestManager_StopUsesOpenedRootAfterPathIsReplaced(t *testing.T) {
|
||||
tempBase := setAgentTempTestEnv(t)
|
||||
mgr := NewManager(&config.InstanceConfig{
|
||||
WorkDir: t.TempDir(),
|
||||
SessionID: "session-replaced-root",
|
||||
}, newTestLogger(t))
|
||||
if err := mgr.ensureAgentTempEnv(); err != nil {
|
||||
t.Fatalf("ensureAgentTempEnv() error = %v", err)
|
||||
}
|
||||
|
||||
root := filepath.Join(tempBase, agentTempDirRoot)
|
||||
parkedRoot := filepath.Join(tempBase, "original-kandev-agent")
|
||||
if err := os.Rename(root, parkedRoot); err != nil {
|
||||
if runtime.GOOS == "windows" {
|
||||
if stopErr := mgr.StopForTeardown(context.Background()); stopErr != nil {
|
||||
t.Fatalf("StopForTeardown() after protected-root rename = %v", stopErr)
|
||||
}
|
||||
return
|
||||
}
|
||||
t.Fatalf("rename original temp root: %v", err)
|
||||
}
|
||||
outside := t.TempDir()
|
||||
outsideSession := filepath.Join(outside, "session-replaced-root")
|
||||
if err := os.MkdirAll(outsideSession, 0o700); err != nil {
|
||||
t.Fatalf("create outside session dir: %v", err)
|
||||
}
|
||||
if err := os.Symlink(outside, root); err != nil {
|
||||
t.Skipf("create replacement symlink fixture: %v", err)
|
||||
}
|
||||
|
||||
ownedChild := mgr.agentTempChild
|
||||
err := mgr.StopForTeardown(context.Background())
|
||||
if err != nil {
|
||||
t.Fatalf("Stop() error = %v", err)
|
||||
}
|
||||
if info, err := os.Stat(outsideSession); err != nil || !info.IsDir() {
|
||||
t.Fatalf("cleanup traversed replaced root, stat = %v, err = %v", info, err)
|
||||
}
|
||||
if _, err := os.Stat(filepath.Join(parkedRoot, ownedChild)); !os.IsNotExist(err) {
|
||||
t.Fatalf("owned child under original root remains after cleanup, err = %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestManager_TempNamesDoNotCollideAfterSanitization(t *testing.T) {
|
||||
setAgentTempTestEnv(t)
|
||||
first := NewManager(&config.InstanceConfig{
|
||||
InstanceID: "instance-first",
|
||||
WorkDir: t.TempDir(),
|
||||
SessionID: "a/b",
|
||||
Port: 41001,
|
||||
}, newTestLogger(t))
|
||||
second := NewManager(&config.InstanceConfig{
|
||||
InstanceID: "instance-second",
|
||||
WorkDir: t.TempDir(),
|
||||
SessionID: "a_b",
|
||||
Port: 41002,
|
||||
}, newTestLogger(t))
|
||||
for _, mgr := range []*Manager{first, second} {
|
||||
if err := mgr.ensureAgentTempEnv(); err != nil {
|
||||
t.Fatalf("ensureAgentTempEnv() error = %v", err)
|
||||
}
|
||||
}
|
||||
if first.agentTempDir == second.agentTempDir {
|
||||
t.Fatalf("distinct instance identities share temp dir %q", first.agentTempDir)
|
||||
}
|
||||
firstDir := first.agentTempDir
|
||||
secondSentinel := filepath.Join(second.agentTempDir, "must-remain")
|
||||
if err := os.WriteFile(secondSentinel, []byte("owned by second"), 0o600); err != nil {
|
||||
t.Fatalf("create second sentinel: %v", err)
|
||||
}
|
||||
|
||||
if err := first.StopForTeardown(context.Background()); err != nil {
|
||||
t.Fatalf("first Stop() error = %v", err)
|
||||
}
|
||||
if _, err := os.Stat(firstDir); !os.IsNotExist(err) {
|
||||
t.Fatalf("first temp dir still exists after Stop(), err = %v", err)
|
||||
}
|
||||
if data, err := os.ReadFile(secondSentinel); err != nil || string(data) != "owned by second" {
|
||||
t.Fatalf("first cleanup changed second temp dir, data = %q, err = %v", data, err)
|
||||
}
|
||||
if err := second.StopForTeardown(context.Background()); err != nil {
|
||||
t.Fatalf("second Stop() error = %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestManager_StoppedAgentKeepsTempUntilWorkspaceProcessReaped(t *testing.T) {
|
||||
setAgentTempTestEnv(t)
|
||||
mgr := NewManager(&config.InstanceConfig{
|
||||
InstanceID: "instance-natural-exit",
|
||||
WorkDir: t.TempDir(),
|
||||
SessionID: "session-natural-exit",
|
||||
}, newTestLogger(t))
|
||||
if err := mgr.ensureAgentTempEnv(); err != nil {
|
||||
t.Fatalf("ensureAgentTempEnv() error = %v", err)
|
||||
}
|
||||
owned := mgr.agentTempDir
|
||||
proc := &commandProcess{
|
||||
info: ProcessInfo{ID: "workspace-blocked-reap"},
|
||||
stopSignal: make(chan struct{}),
|
||||
done: make(chan struct{}),
|
||||
}
|
||||
mgr.processRunner.processes[proc.info.ID] = proc
|
||||
|
||||
stopDone := make(chan error, 1)
|
||||
go func() { stopDone <- mgr.StopForTeardown(context.Background()) }()
|
||||
select {
|
||||
case <-proc.stopSignal:
|
||||
case err := <-stopDone:
|
||||
t.Fatalf("Stop() returned before workspace process stop/reap: %v", err)
|
||||
}
|
||||
if info, err := os.Stat(owned); err != nil || !info.IsDir() {
|
||||
t.Fatalf("temp dir removed before workspace process reap, stat = %v, err = %v", info, err)
|
||||
}
|
||||
|
||||
close(proc.done)
|
||||
if err := <-stopDone; err != nil {
|
||||
t.Fatalf("Stop() error = %v", err)
|
||||
}
|
||||
if _, err := os.Stat(owned); !os.IsNotExist(err) {
|
||||
t.Fatalf("temp dir still exists after workspace process reap, err = %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestManager_BeginStopWaitsForInFlightTempOwnerAdmission(t *testing.T) {
|
||||
func TestManager_BeginStopWaitsForInFlightAdmission(t *testing.T) {
|
||||
mgr := NewManager(&config.InstanceConfig{WorkDir: t.TempDir()}, newTestLogger(t))
|
||||
release, err := mgr.admitStart()
|
||||
if err != nil {
|
||||
@@ -431,7 +99,7 @@ func TestManager_BeginStopWaitsForInFlightTempOwnerAdmission(t *testing.T) {
|
||||
}()
|
||||
select {
|
||||
case <-stopAdmissionDone:
|
||||
t.Fatal("BeginStop() returned before in-flight owner admission completed")
|
||||
t.Fatal("BeginStop() returned before in-flight admission completed")
|
||||
default:
|
||||
}
|
||||
release()
|
||||
@@ -454,55 +122,8 @@ func TestManager_BeginStopWaitsForInFlightTempOwnerAdmission(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestManager_OrdinaryStopCannotCleanTempDuringTerminalAdmissionDrain(t *testing.T) {
|
||||
setAgentTempTestEnv(t)
|
||||
mgr := NewManager(&config.InstanceConfig{
|
||||
WorkDir: t.TempDir(),
|
||||
SessionID: "session-concurrent-stop",
|
||||
}, newTestLogger(t))
|
||||
if err := mgr.ensureAgentTempEnv(); err != nil {
|
||||
t.Fatalf("ensureAgentTempEnv() error = %v", err)
|
||||
}
|
||||
owned := mgr.agentTempDir
|
||||
release, err := mgr.admitStart()
|
||||
if err != nil {
|
||||
t.Fatalf("admitStart() error = %v", err)
|
||||
}
|
||||
mgr.CloseAdmission()
|
||||
|
||||
if err := mgr.Stop(context.Background()); err != nil {
|
||||
t.Fatalf("ordinary Stop() error = %v", err)
|
||||
}
|
||||
if info, err := os.Stat(owned); err != nil || !info.IsDir() {
|
||||
t.Fatalf("ordinary Stop() removed terminal-owned temp dir, stat = %v, err = %v", info, err)
|
||||
}
|
||||
|
||||
terminalDone := make(chan error, 1)
|
||||
go func() { terminalDone <- mgr.StopForTeardown(context.Background()) }()
|
||||
select {
|
||||
case err := <-terminalDone:
|
||||
t.Fatalf("terminal stop returned before admission drained: %v", err)
|
||||
default:
|
||||
}
|
||||
release()
|
||||
if err := <-terminalDone; err != nil {
|
||||
t.Fatalf("StopForTeardown() error = %v", err)
|
||||
}
|
||||
if _, err := os.Stat(owned); !os.IsNotExist(err) {
|
||||
t.Fatalf("terminal stop did not remove temp dir after drain, err = %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestManager_TerminalAdmissionDrainHonorsContextAndRetries(t *testing.T) {
|
||||
setAgentTempTestEnv(t)
|
||||
mgr := NewManager(&config.InstanceConfig{
|
||||
WorkDir: t.TempDir(),
|
||||
SessionID: "session-drain-timeout",
|
||||
}, newTestLogger(t))
|
||||
if err := mgr.ensureAgentTempEnv(); err != nil {
|
||||
t.Fatalf("ensureAgentTempEnv() error = %v", err)
|
||||
}
|
||||
owned := mgr.agentTempDir
|
||||
func TestManager_TeardownAdmissionDrainHonorsContextAndRetries(t *testing.T) {
|
||||
mgr := NewManager(&config.InstanceConfig{WorkDir: t.TempDir()}, newTestLogger(t))
|
||||
release, err := mgr.admitStart()
|
||||
if err != nil {
|
||||
t.Fatalf("admitStart() error = %v", err)
|
||||
@@ -514,30 +135,43 @@ func TestManager_TerminalAdmissionDrainHonorsContextAndRetries(t *testing.T) {
|
||||
if !errors.Is(err, context.Canceled) {
|
||||
t.Fatalf("StopForTeardown() error = %v, want %v", err, context.Canceled)
|
||||
}
|
||||
if info, statErr := os.Stat(owned); statErr != nil || !info.IsDir() {
|
||||
t.Fatalf("canceled drain removed temp ownership, stat = %v, err = %v", info, statErr)
|
||||
}
|
||||
|
||||
release()
|
||||
if err := mgr.StopForTeardown(context.Background()); err != nil {
|
||||
t.Fatalf("StopForTeardown() retry error = %v", err)
|
||||
}
|
||||
if _, err := os.Stat(owned); !os.IsNotExist(err) {
|
||||
t.Fatalf("retry did not remove temp dir, err = %v", err)
|
||||
}
|
||||
|
||||
func TestManager_TeardownWaitsForWorkspaceProcessReap(t *testing.T) {
|
||||
mgr := NewManager(&config.InstanceConfig{WorkDir: t.TempDir()}, newTestLogger(t))
|
||||
proc := &commandProcess{
|
||||
info: ProcessInfo{ID: "workspace-blocked-reap"},
|
||||
stopSignal: make(chan struct{}),
|
||||
done: make(chan struct{}),
|
||||
}
|
||||
mgr.processRunner.processes[proc.info.ID] = proc
|
||||
|
||||
stopDone := make(chan error, 1)
|
||||
go func() { stopDone <- mgr.StopForTeardown(context.Background()) }()
|
||||
select {
|
||||
case <-proc.stopSignal:
|
||||
case err := <-stopDone:
|
||||
t.Fatalf("StopForTeardown() returned before workspace process stop/reap: %v", err)
|
||||
}
|
||||
select {
|
||||
case err := <-stopDone:
|
||||
t.Fatalf("StopForTeardown() returned before workspace process reap: %v", err)
|
||||
default:
|
||||
}
|
||||
|
||||
close(proc.done)
|
||||
if err := <-stopDone; err != nil {
|
||||
t.Fatalf("StopForTeardown() error = %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestManager_TempCleanupWaitsForMainProcessGroupReap(t *testing.T) {
|
||||
setAgentTempTestEnv(t)
|
||||
mgr := NewManager(&config.InstanceConfig{
|
||||
InstanceID: "instance-main-reap-failure",
|
||||
WorkDir: t.TempDir(),
|
||||
SessionID: "session-main-reap-failure",
|
||||
}, newTestLogger(t))
|
||||
if err := mgr.ensureAgentTempEnv(); err != nil {
|
||||
t.Fatalf("ensureAgentTempEnv() error = %v", err)
|
||||
}
|
||||
owned := mgr.agentTempDir
|
||||
func TestManager_TeardownReportsMainProcessGroupReapFailure(t *testing.T) {
|
||||
mgr := NewManager(&config.InstanceConfig{WorkDir: t.TempDir()}, newTestLogger(t))
|
||||
mgr.cmd = &exec.Cmd{Process: &os.Process{Pid: 424242}}
|
||||
mgr.status.Store(StatusRunning)
|
||||
mgr.groupAliveFn = func(int) bool { return true }
|
||||
@@ -549,26 +183,15 @@ func TestManager_TempCleanupWaitsForMainProcessGroupReap(t *testing.T) {
|
||||
mgr.waitGroupExitFn = nil
|
||||
_ = mgr.StopForTeardown(context.Background())
|
||||
})
|
||||
|
||||
err := mgr.StopForTeardown(context.Background())
|
||||
if err == nil || !strings.Contains(err.Error(), "remains alive") {
|
||||
t.Fatalf("Stop() error = %v, want process-group reap failure", err)
|
||||
}
|
||||
if info, statErr := os.Stat(owned); statErr != nil || !info.IsDir() {
|
||||
t.Fatalf("temp dir removed after failed main reap, stat = %v, err = %v", info, statErr)
|
||||
t.Fatalf("StopForTeardown() error = %v, want process-group reap failure", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestManager_TempCleanupWaitsForMainGoroutineReap(t *testing.T) {
|
||||
setAgentTempTestEnv(t)
|
||||
mgr := NewManager(&config.InstanceConfig{
|
||||
InstanceID: "instance-main-wait-failure",
|
||||
WorkDir: t.TempDir(),
|
||||
SessionID: "session-main-wait-failure",
|
||||
}, newTestLogger(t))
|
||||
if err := mgr.ensureAgentTempEnv(); err != nil {
|
||||
t.Fatalf("ensureAgentTempEnv() error = %v", err)
|
||||
}
|
||||
owned := mgr.agentTempDir
|
||||
func TestManager_TeardownReportsMainGoroutineReapFailure(t *testing.T) {
|
||||
mgr := NewManager(&config.InstanceConfig{WorkDir: t.TempDir()}, newTestLogger(t))
|
||||
mgr.cmd = &exec.Cmd{Process: &os.Process{Pid: 424243}}
|
||||
mgr.status.Store(StatusRunning)
|
||||
mgr.wg.Add(1)
|
||||
@@ -581,16 +204,14 @@ func TestManager_TempCleanupWaitsForMainGoroutineReap(t *testing.T) {
|
||||
mgr.terminateGroupFn = func(int) error { return nil }
|
||||
mgr.killGroupFn = func(int) error { return nil }
|
||||
mgr.managerWaitFn = func(context.Context, <-chan struct{}, time.Duration) bool { return false }
|
||||
|
||||
err := mgr.StopForTeardown(context.Background())
|
||||
if err == nil || !strings.Contains(err.Error(), "goroutines were not reaped") {
|
||||
t.Fatalf("Stop() error = %v, want manager reap failure", err)
|
||||
}
|
||||
if info, statErr := os.Stat(owned); statErr != nil || !info.IsDir() {
|
||||
t.Fatalf("temp dir removed after failed manager reap, stat = %v, err = %v", info, statErr)
|
||||
t.Fatalf("StopForTeardown() error = %v, want manager reap failure", err)
|
||||
}
|
||||
}
|
||||
|
||||
func setAgentTempTestEnv(t *testing.T) string {
|
||||
func setServiceTempTestEnv(t *testing.T) string {
|
||||
t.Helper()
|
||||
root := t.TempDir()
|
||||
for _, key := range []string{"TMPDIR", "TMP", "TEMP"} {
|
||||
@@ -598,3 +219,10 @@ func setAgentTempTestEnv(t *testing.T) string {
|
||||
}
|
||||
return root
|
||||
}
|
||||
|
||||
func assertNoAgentTempRoot(t *testing.T, serviceTemp string) {
|
||||
t.Helper()
|
||||
if _, err := os.Stat(filepath.Join(serviceTemp, "kandev-agent")); !os.IsNotExist(err) {
|
||||
t.Fatalf("unexpected kandev-agent root, err = %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,7 +6,6 @@ import (
|
||||
"io"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"slices"
|
||||
"strings"
|
||||
"syscall"
|
||||
@@ -232,21 +231,28 @@ func TestBuildAdapterConfig_StripEnvRemovesDeclaredVars(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestStartOneShotRestoresTempEnvAfterStrip(t *testing.T) {
|
||||
func TestStartOneShotPreservesConfiguredTempEnvironment(t *testing.T) {
|
||||
log := newTestLogger(t)
|
||||
workDir := t.TempDir()
|
||||
serviceTemp := setServiceTempTestEnv(t)
|
||||
agentEnv := []string{
|
||||
"PATH=/usr/bin",
|
||||
"TMPDIR=/configured/tmpdir",
|
||||
"TMP=/configured/tmp",
|
||||
"TEMP=/configured/temp",
|
||||
}
|
||||
|
||||
m := &Manager{
|
||||
cfg: &config.InstanceConfig{
|
||||
SessionID: "session-1",
|
||||
WorkDir: workDir,
|
||||
AgentEnv: []string{"PATH=/usr/bin"},
|
||||
AgentEnv: agentEnv,
|
||||
},
|
||||
logger: log,
|
||||
adapter: newOneShotStubAdapter(),
|
||||
adapterCfg: &adapter.Config{
|
||||
OneShotConfig: &adapter.OneShotConfig{
|
||||
Env: []string{"PATH=/usr/bin"},
|
||||
Env: agentEnv,
|
||||
WorkDir: workDir,
|
||||
},
|
||||
},
|
||||
@@ -262,10 +268,15 @@ func TestStartOneShotRestoresTempEnvAfterStrip(t *testing.T) {
|
||||
m.workspaceTracker.Stop()
|
||||
})
|
||||
|
||||
wantDir := filepath.Join(os.TempDir(), agentTempDirRoot, agentTempDirName("session-1", "", 0))
|
||||
for _, key := range []string{"TMPDIR", "TMP", "TEMP"} {
|
||||
if got := lookupEnvValue(m.adapterCfg.OneShotConfig.Env, key); got != wantDir {
|
||||
t.Fatalf("OneShotConfig.Env %s = %q, want %q", key, got, wantDir)
|
||||
want := map[string]string{
|
||||
"TMPDIR": "/configured/tmpdir",
|
||||
"TMP": "/configured/tmp",
|
||||
"TEMP": "/configured/temp",
|
||||
}
|
||||
for key, value := range want {
|
||||
if got := lookupEnvValue(m.adapterCfg.OneShotConfig.Env, key); got != value {
|
||||
t.Fatalf("OneShotConfig.Env %s = %q, want configured service value %q", key, got, value)
|
||||
}
|
||||
}
|
||||
assertNoAgentTempRoot(t, serviceTemp)
|
||||
}
|
||||
|
||||
@@ -582,17 +582,17 @@ func TestBuildShellEnv(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildShellEnvOverridesManagedTemp(t *testing.T) {
|
||||
func TestBuildShellEnvAppliesExplicitTempOverrides(t *testing.T) {
|
||||
workDir := t.TempDir()
|
||||
t.Setenv("TMPDIR", "unmanaged")
|
||||
env := buildShellEnv(workDir, map[string]string{
|
||||
"TMPDIR": "/tmp/kandev-agent/session",
|
||||
"TMP": "/tmp/kandev-agent/session",
|
||||
"TEMP": "/tmp/kandev-agent/session",
|
||||
"TMPDIR": "/srv/kandev/shared-tmp",
|
||||
"TMP": "/srv/kandev/shared-tmp",
|
||||
"TEMP": "/srv/kandev/shared-tmp",
|
||||
})
|
||||
|
||||
for _, key := range []string{"TMPDIR", "TMP", "TEMP"} {
|
||||
want := key + "=/tmp/kandev-agent/session"
|
||||
want := key + "=/srv/kandev/shared-tmp"
|
||||
found := false
|
||||
for _, entry := range env {
|
||||
if entry == want {
|
||||
|
||||
@@ -22,6 +22,9 @@ import (
|
||||
const (
|
||||
issueWatchIDKey = "issue_watch_id"
|
||||
issueNumberKey = "issue_number"
|
||||
githubPRStateOpen = "open"
|
||||
githubPRStateClosed = "closed"
|
||||
githubPRStateMerged = "merged"
|
||||
ciAutomationDetachedTimeout = 2 * time.Minute
|
||||
)
|
||||
|
||||
|
||||
@@ -446,14 +446,14 @@ func (s *Service) recordCIAutomationError(ctx context.Context, pr *github.TaskPR
|
||||
}
|
||||
|
||||
func ciAutomationCanAutoFixFromFeedback(pr *github.TaskPR) bool {
|
||||
return pr != nil && pr.State != "closed" && pr.State != "merged"
|
||||
return pr != nil && pr.State != githubPRStateClosed && pr.State != githubPRStateMerged
|
||||
}
|
||||
|
||||
func ciAutomationCanAutoFixFromFeedbackPR(feedback *github.PRFeedback) bool {
|
||||
if feedback == nil || feedback.PR == nil {
|
||||
return true
|
||||
}
|
||||
return feedback.PR.State != "closed" && feedback.PR.State != "merged"
|
||||
return feedback.PR.State != githubPRStateClosed && feedback.PR.State != githubPRStateMerged
|
||||
}
|
||||
|
||||
func ciAutomationChecksSettledForAutoFix(pr *github.TaskPR, feedback *github.PRFeedback) bool {
|
||||
@@ -517,7 +517,7 @@ func ciAutomationShouldIncludeFeedbackComment(pr *github.TaskPR, comment github.
|
||||
}
|
||||
|
||||
func ciAutomationReadyToMerge(pr *github.TaskPR) bool {
|
||||
if pr == nil || pr.State != "open" {
|
||||
if pr == nil || pr.State != githubPRStateOpen {
|
||||
return false
|
||||
}
|
||||
if pr.ChecksState != ciAutomationCheckSuccess || pr.MergeableState != "clean" {
|
||||
|
||||
@@ -1610,7 +1610,7 @@ func taskPRState(prs []*github.TaskPR, repositoryID, branch string) (string, boo
|
||||
}
|
||||
matched = true
|
||||
state = strings.ToLower(strings.TrimSpace(pr.State))
|
||||
if state != "open" && state != "closed" && state != "merged" {
|
||||
if state != githubPRStateOpen && state != githubPRStateClosed && state != githubPRStateMerged {
|
||||
return "", true
|
||||
}
|
||||
}
|
||||
@@ -1642,14 +1642,14 @@ func (s *Service) handleSessionLaunchFailed(ctx context.Context, taskID, session
|
||||
|
||||
branch := extractMissingBranchName(launchErr)
|
||||
prState := s.matchingTaskPRState(ctx, taskID, repositoryID, branch)
|
||||
if prState == "open" {
|
||||
if prState == githubPRStateOpen {
|
||||
return
|
||||
}
|
||||
content := "Kandev couldn't fetch the requested branch from the configured repository. Verify the task's repository branch or PR link, then retry."
|
||||
if branch != "" {
|
||||
content = "Kandev couldn't fetch branch \"" + branch + "\" from the configured repository. Verify the task's repository branch or PR link, then retry."
|
||||
}
|
||||
authoritativeMissingBranch := prState == "closed" || prState == "merged"
|
||||
authoritativeMissingBranch := prState == githubPRStateClosed || prState == githubPRStateMerged
|
||||
if authoritativeMissingBranch {
|
||||
content = "This task references a PR branch that no longer exists on remote."
|
||||
if branch != "" {
|
||||
|
||||
+250
-165
@@ -50,23 +50,57 @@ export type BackendContext = {
|
||||
restart: (envOverrides?: Record<string, string>) => Promise<void>;
|
||||
};
|
||||
|
||||
async function waitForHealth(url: string, timeoutMs: number, proc?: ChildProcess): Promise<void> {
|
||||
const deadline = Date.now() + timeoutMs;
|
||||
|
||||
// Track process exit so we can fail fast instead of polling for the full timeout.
|
||||
let processExited = false;
|
||||
let processExitCode: number | null = null;
|
||||
function observeProcessExit(proc?: ChildProcess): {
|
||||
state: { exited: boolean; exitCode: number | null; error?: Error };
|
||||
dispose: () => void;
|
||||
} {
|
||||
const state: { exited: boolean; exitCode: number | null; error?: Error } = {
|
||||
exited: proc?.exitCode !== null && proc?.exitCode !== undefined,
|
||||
exitCode: proc?.exitCode ?? null,
|
||||
};
|
||||
const onExit = (code: number | null) => {
|
||||
processExited = true;
|
||||
processExitCode = code;
|
||||
state.exited = true;
|
||||
state.exitCode = code;
|
||||
};
|
||||
const onError = (error: Error) => {
|
||||
state.error = error;
|
||||
};
|
||||
proc?.once("exit", onExit);
|
||||
proc?.once("error", onError);
|
||||
|
||||
// Close the gap between inspecting exitCode and subscribing to the event.
|
||||
// Node records exitCode before emitting `exit`, so this second read catches
|
||||
// a child that exited between those two operations.
|
||||
if (proc?.exitCode !== null && proc?.exitCode !== undefined) {
|
||||
state.exited = true;
|
||||
state.exitCode = proc.exitCode;
|
||||
}
|
||||
|
||||
return {
|
||||
state,
|
||||
dispose: () => {
|
||||
proc?.off("exit", onExit);
|
||||
proc?.off("error", onError);
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
export async function waitForHealth(
|
||||
url: string,
|
||||
timeoutMs: number,
|
||||
proc?: ChildProcess,
|
||||
): Promise<void> {
|
||||
const deadline = Date.now() + timeoutMs;
|
||||
const processExit = observeProcessExit(proc);
|
||||
|
||||
try {
|
||||
while (Date.now() < deadline) {
|
||||
if (processExited) {
|
||||
if (processExit.state.error) {
|
||||
throw processExit.state.error;
|
||||
}
|
||||
if (processExit.state.exited) {
|
||||
throw new Error(
|
||||
`Backend process exited with code ${processExitCode} while waiting for health at ${url}`,
|
||||
`Backend process exited with code ${processExit.state.exitCode} while waiting for health at ${url}`,
|
||||
);
|
||||
}
|
||||
try {
|
||||
@@ -79,7 +113,7 @@ async function waitForHealth(url: string, timeoutMs: number, proc?: ChildProcess
|
||||
}
|
||||
throw new Error(`Service did not become healthy at ${url} within ${timeoutMs}ms`);
|
||||
} finally {
|
||||
proc?.off("exit", onExit);
|
||||
processExit.dispose();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -147,6 +181,61 @@ function killProcessGroup(proc: ChildProcess): Promise<void> {
|
||||
});
|
||||
}
|
||||
|
||||
type BackendFixtureLifecycle = {
|
||||
stopProcess?: (proc: ChildProcess) => Promise<void>;
|
||||
removeTempRoot?: (tmpDir: string) => void;
|
||||
};
|
||||
|
||||
function removeOwnedTempRoot(tmpDir: string): void {
|
||||
fs.rmSync(tmpDir, {
|
||||
recursive: true,
|
||||
force: true,
|
||||
maxRetries: 3,
|
||||
retryDelay: 100,
|
||||
});
|
||||
}
|
||||
|
||||
export async function runOwnedBackendFixture<T>(
|
||||
tmpDir: string,
|
||||
run: (registerProcess: (proc: ChildProcess) => void) => Promise<T>,
|
||||
lifecycle: BackendFixtureLifecycle = {},
|
||||
): Promise<T> {
|
||||
const stopProcess = lifecycle.stopProcess ?? killProcessGroup;
|
||||
const removeTempRoot = lifecycle.removeTempRoot ?? removeOwnedTempRoot;
|
||||
let backendProc: ChildProcess | undefined;
|
||||
let result: T | undefined;
|
||||
const failures: unknown[] = [];
|
||||
|
||||
try {
|
||||
result = await run((proc) => {
|
||||
backendProc = proc;
|
||||
});
|
||||
} catch (error) {
|
||||
failures.push(error);
|
||||
}
|
||||
|
||||
if (backendProc) {
|
||||
try {
|
||||
await stopProcess(backendProc);
|
||||
} catch (error) {
|
||||
failures.push(error);
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
removeTempRoot(tmpDir);
|
||||
} catch (error) {
|
||||
failures.push(new Error(`Failed to remove E2E temporary root ${tmpDir}`, { cause: error }));
|
||||
}
|
||||
|
||||
if (failures.length === 1) throw failures[0];
|
||||
if (failures.length > 1) {
|
||||
throw new AggregateError(failures, "Backend fixture failed and cleanup did not complete");
|
||||
}
|
||||
|
||||
return result as T;
|
||||
}
|
||||
|
||||
/**
|
||||
* Spawn a backend process with the given environment. Returns the child process.
|
||||
* The process is spawned with `detached: true` so it becomes a process group leader.
|
||||
@@ -197,51 +286,54 @@ export const backendFixture = base.extend<object, { backend: BackendContext }>({
|
||||
const tmpDir = fs.mkdtempSync(
|
||||
path.join(os.tmpdir(), `kandev-e2e-${workerInfo.workerIndex}-`),
|
||||
);
|
||||
const homeDir = path.join(tmpDir, ".kandev");
|
||||
const dbPath = path.join(tmpDir, "kandev.db");
|
||||
const worktreeBase = path.join(tmpDir, "worktrees");
|
||||
const repoCloneBase = path.join(tmpDir, "repos");
|
||||
let backendProc: ChildProcess | undefined;
|
||||
|
||||
fs.mkdirSync(homeDir, { recursive: true });
|
||||
fs.mkdirSync(worktreeBase, { recursive: true });
|
||||
fs.mkdirSync(repoCloneBase, { recursive: true });
|
||||
await runOwnedBackendFixture(tmpDir, async (registerProcess) => {
|
||||
const homeDir = path.join(tmpDir, ".kandev");
|
||||
const dbPath = path.join(tmpDir, "kandev.db");
|
||||
const worktreeBase = path.join(tmpDir, "worktrees");
|
||||
const repoCloneBase = path.join(tmpDir, "repos");
|
||||
|
||||
// Write a minimal .gitconfig so git doesn't prompt for identity
|
||||
// and disable signing to avoid SSH/GPG key lookups in the isolated HOME.
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, ".gitconfig"),
|
||||
"[user]\n name = E2E Test\n email = e2e@test.local\n[commit]\n gpgsign = false\n[tag]\n gpgsign = false\n",
|
||||
);
|
||||
fs.mkdirSync(homeDir, { recursive: true });
|
||||
fs.mkdirSync(worktreeBase, { recursive: true });
|
||||
fs.mkdirSync(repoCloneBase, { recursive: true });
|
||||
|
||||
// Give each worker its own agentctl port range, offset from the default
|
||||
// range (41001-41100) to avoid conflicts with a running dev instance.
|
||||
// The async cleanup of agent instances runs after each test deletes its
|
||||
// tasks, so during a 60+ test shard the in-flight cleanup queue can hold
|
||||
// several dozen ports at any given moment. 200 ports per worker keeps
|
||||
// headroom for that without overflowing the 65535 port space. With the
|
||||
// current playwright config (workers: 1, so workerIndex == 0) and shard
|
||||
// offsets capped at 29, the highest port used is 30001 + 29*1000 + 199
|
||||
// = 59200. The `workerIndex * 200` term is defensive for the case where
|
||||
// a future config sets workers > 1.
|
||||
const agentctlPortBase = 30001 + E2E_PORT_OFFSET * 1000 + workerInfo.workerIndex * 200;
|
||||
const agentctlPortMax = agentctlPortBase + 199;
|
||||
// Write a minimal .gitconfig so git doesn't prompt for identity
|
||||
// and disable signing to avoid SSH/GPG key lookups in the isolated HOME.
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, ".gitconfig"),
|
||||
"[user]\n name = E2E Test\n email = e2e@test.local\n[commit]\n gpgsign = false\n[tag]\n gpgsign = false\n",
|
||||
);
|
||||
|
||||
// Install a `git` shim that can sleep on `fetch`/`pull` before execing
|
||||
// the real git binary. Tests that need to simulate slow network git
|
||||
// operations write a millisecond value to `${tmpDir}/git-delay-ms`; the
|
||||
// shim reads it on every invocation and sleeps the matching duration.
|
||||
// When the file is absent the shim is a transparent passthrough, so
|
||||
// other tests in the same worker are unaffected.
|
||||
const shimDir = path.join(tmpDir, "bin");
|
||||
const shimPath = path.join(shimDir, "git");
|
||||
const shimDelayFile = path.join(tmpDir, "git-delay-ms");
|
||||
const shimGitLabPushFile = path.join(tmpDir, "gitlab-push-remote");
|
||||
const shimGitLabPushRecordFile = path.join(tmpDir, "gitlab-push-record");
|
||||
const originalPath = process.env.PATH ?? "";
|
||||
fs.mkdirSync(shimDir, { recursive: true });
|
||||
fs.writeFileSync(
|
||||
shimPath,
|
||||
`#!/bin/sh
|
||||
// Give each worker its own agentctl port range, offset from the default
|
||||
// range (41001-41100) to avoid conflicts with a running dev instance.
|
||||
// The async cleanup of agent instances runs after each test deletes its
|
||||
// tasks, so during a 60+ test shard the in-flight cleanup queue can hold
|
||||
// several dozen ports at any given moment. 200 ports per worker keeps
|
||||
// headroom for that without overflowing the 65535 port space. With the
|
||||
// current playwright config (workers: 1, so workerIndex == 0) and shard
|
||||
// offsets capped at 29, the highest port used is 30001 + 29*1000 + 199
|
||||
// = 59200. The `workerIndex * 200` term is defensive for the case where
|
||||
// a future config sets workers > 1.
|
||||
const agentctlPortBase = 30001 + E2E_PORT_OFFSET * 1000 + workerInfo.workerIndex * 200;
|
||||
const agentctlPortMax = agentctlPortBase + 199;
|
||||
|
||||
// Install a `git` shim that can sleep on `fetch`/`pull` before execing
|
||||
// the real git binary. Tests that need to simulate slow network git
|
||||
// operations write a millisecond value to `${tmpDir}/git-delay-ms`; the
|
||||
// shim reads it on every invocation and sleeps the matching duration.
|
||||
// When the file is absent the shim is a transparent passthrough, so
|
||||
// other tests in the same worker are unaffected.
|
||||
const shimDir = path.join(tmpDir, "bin");
|
||||
const shimPath = path.join(shimDir, "git");
|
||||
const shimDelayFile = path.join(tmpDir, "git-delay-ms");
|
||||
const shimGitLabPushFile = path.join(tmpDir, "gitlab-push-remote");
|
||||
const shimGitLabPushRecordFile = path.join(tmpDir, "gitlab-push-record");
|
||||
const originalPath = process.env.PATH ?? "";
|
||||
fs.mkdirSync(shimDir, { recursive: true });
|
||||
fs.writeFileSync(
|
||||
shimPath,
|
||||
`#!/bin/sh
|
||||
if [ -f "$KANDEV_E2E_GIT_DELAY_FILE" ] && { [ "$1" = "fetch" ] || [ "$1" = "pull" ]; }; then
|
||||
delay_ms=$(cat "$KANDEV_E2E_GIT_DELAY_FILE" 2>/dev/null)
|
||||
case "$delay_ms" in
|
||||
@@ -266,127 +358,120 @@ fi
|
||||
export PATH="$KANDEV_E2E_ORIGINAL_PATH"
|
||||
exec git "$@"
|
||||
`,
|
||||
{ mode: 0o755 },
|
||||
);
|
||||
{ mode: 0o755 },
|
||||
);
|
||||
|
||||
// Opt-in: Docker E2E project or KANDEV_E2E_DOCKER=1 enables real
|
||||
// container execution. Default is off so the regular suite stays fast
|
||||
// and runs without a Docker daemon. See e2e/README.md.
|
||||
const dockerEnabled = isContainerProjectActive(workerInfo.project.name);
|
||||
const mockAgentLinuxBinary = path.join(BACKEND_DIR, "bin", "mock-agent-linux-amd64");
|
||||
const agentctlLinuxBinary = path.join(BACKEND_DIR, "bin", "agentctl-linux-amd64");
|
||||
// Opt-in: Docker E2E project or KANDEV_E2E_DOCKER=1 enables real
|
||||
// container execution. Default is off so the regular suite stays fast
|
||||
// and runs without a Docker daemon. See e2e/README.md.
|
||||
const dockerEnabled = isContainerProjectActive(workerInfo.project.name);
|
||||
const mockAgentLinuxBinary = path.join(BACKEND_DIR, "bin", "mock-agent-linux-amd64");
|
||||
const agentctlLinuxBinary = path.join(BACKEND_DIR, "bin", "agentctl-linux-amd64");
|
||||
|
||||
const backendEnv = {
|
||||
...sanitizeInheritedEnv(process.env as Record<string, string>),
|
||||
// Prepend the kandev bin dir so the host utility probe can locate
|
||||
// the `mock-agent` binary via PATH. In production that dir is the
|
||||
// same as the running kandev binary's dir, but e2e spawns via an
|
||||
// absolute path and doesn't inherit that location.
|
||||
PATH: `${shimDir}:${path.join(BACKEND_DIR, "bin")}:${originalPath}`,
|
||||
KANDEV_E2E_ORIGINAL_PATH: originalPath,
|
||||
KANDEV_E2E_GIT_DELAY_FILE: shimDelayFile,
|
||||
KANDEV_E2E_GITLAB_PUSH_FILE: shimGitLabPushFile,
|
||||
KANDEV_E2E_GITLAB_PUSH_RECORD_FILE: shimGitLabPushRecordFile,
|
||||
KANDEV_E2E_GITLAB_REMOTE_URL: `http://localhost:${backendPort}/platform/kandev.git`,
|
||||
HOME: tmpDir,
|
||||
KANDEV_HOME_DIR: homeDir,
|
||||
KANDEV_SERVER_PORT: String(backendPort),
|
||||
KANDEV_WEB_DIST_DIR: WEB_DIST_DIR,
|
||||
KANDEV_DATABASE_PATH: dbPath,
|
||||
// Profile selector. KANDEV_E2E_MOCK=true tells the backend to
|
||||
// apply the `e2e:` profile from profiles.yaml at startup —
|
||||
// which sets the mock agent and third-party provider flags,
|
||||
// KANDEV_FEATURES_OFFICE, AGENTCTL_AUTO_APPROVE_PERMISSIONS,
|
||||
// KANDEV_PLAN_COALESCE_WINDOW_MS, etc. We don't re-set those
|
||||
// here. KANDEV_MOCK_PROVIDERS stays opt-in per-spec because it
|
||||
// changes agent counts; the five office-routing-* specs pass
|
||||
// it to backend.restart() when needed (see
|
||||
// registry.RoutableProviderIDs).
|
||||
KANDEV_E2E_MOCK: "true",
|
||||
KANDEV_DOCKER_ENABLED: dockerEnabled ? "true" : "false",
|
||||
// When Docker is on, point the lifecycle resolvers at the linux/amd64
|
||||
// binaries the test runner pre-built, so containers can bind-mount them.
|
||||
...(dockerEnabled
|
||||
? {
|
||||
KANDEV_AGENTCTL_LINUX_BINARY: agentctlLinuxBinary,
|
||||
KANDEV_MOCK_AGENT_LINUX_BINARY: mockAgentLinuxBinary,
|
||||
}
|
||||
: {}),
|
||||
KANDEV_WORKTREE_ENABLED: "true",
|
||||
KANDEV_WORKTREE_BASEPATH: worktreeBase,
|
||||
KANDEV_REPOCLONE_BASEPATH: repoCloneBase,
|
||||
KANDEV_LOG_LEVEL: process.env.KANDEV_LOG_LEVEL ?? "warn",
|
||||
AGENTCTL_INSTANCE_PORT_BASE: String(agentctlPortBase),
|
||||
AGENTCTL_INSTANCE_PORT_MAX: String(agentctlPortMax),
|
||||
// AGENTCTL_AUTO_APPROVE_PERMISSIONS=true and
|
||||
// KANDEV_PLAN_COALESCE_WINDOW_MS=2000 are applied by the
|
||||
// backend's profile loader (profiles.yaml `e2e:` column).
|
||||
// Specs that need different values (e.g.
|
||||
// permission-approval.spec.ts setting auto-approve=false) set
|
||||
// process.env.X before spawn — that already flows through the
|
||||
// `...sanitizeInheritedEnv(process.env)` spread above, and the
|
||||
// backend's ApplyProfile leaves already-set vars alone. (Note:
|
||||
// KANDEV_FEATURES_* is the exception — it's stripped from the
|
||||
// inherited env so the profile always governs feature flags.)
|
||||
GIT_AUTHOR_NAME: "E2E Test",
|
||||
GIT_AUTHOR_EMAIL: "e2e@test.local",
|
||||
GIT_COMMITTER_NAME: "E2E Test",
|
||||
GIT_COMMITTER_EMAIL: "e2e@test.local",
|
||||
};
|
||||
const backendEnv = {
|
||||
...sanitizeInheritedEnv(process.env as Record<string, string>),
|
||||
// Prepend the kandev bin dir so the host utility probe can locate
|
||||
// the `mock-agent` binary via PATH. In production that dir is the
|
||||
// same as the running kandev binary's dir, but e2e spawns via an
|
||||
// absolute path and doesn't inherit that location.
|
||||
PATH: `${shimDir}:${path.join(BACKEND_DIR, "bin")}:${originalPath}`,
|
||||
KANDEV_E2E_ORIGINAL_PATH: originalPath,
|
||||
KANDEV_E2E_GIT_DELAY_FILE: shimDelayFile,
|
||||
KANDEV_E2E_GITLAB_PUSH_FILE: shimGitLabPushFile,
|
||||
KANDEV_E2E_GITLAB_PUSH_RECORD_FILE: shimGitLabPushRecordFile,
|
||||
KANDEV_E2E_GITLAB_REMOTE_URL: `http://localhost:${backendPort}/platform/kandev.git`,
|
||||
HOME: tmpDir,
|
||||
KANDEV_HOME_DIR: homeDir,
|
||||
KANDEV_SERVER_PORT: String(backendPort),
|
||||
KANDEV_WEB_DIST_DIR: WEB_DIST_DIR,
|
||||
KANDEV_DATABASE_PATH: dbPath,
|
||||
// Profile selector. KANDEV_E2E_MOCK=true tells the backend to
|
||||
// apply the `e2e:` profile from profiles.yaml at startup —
|
||||
// which sets the mock agent and third-party provider flags,
|
||||
// KANDEV_FEATURES_OFFICE, AGENTCTL_AUTO_APPROVE_PERMISSIONS,
|
||||
// KANDEV_PLAN_COALESCE_WINDOW_MS, etc. We don't re-set those
|
||||
// here. KANDEV_MOCK_PROVIDERS stays opt-in per-spec because it
|
||||
// changes agent counts; the five office-routing-* specs pass
|
||||
// it to backend.restart() when needed (see
|
||||
// registry.RoutableProviderIDs).
|
||||
KANDEV_E2E_MOCK: "true",
|
||||
KANDEV_DOCKER_ENABLED: dockerEnabled ? "true" : "false",
|
||||
// When Docker is on, point the lifecycle resolvers at the linux/amd64
|
||||
// binaries the test runner pre-built, so containers can bind-mount them.
|
||||
...(dockerEnabled
|
||||
? {
|
||||
KANDEV_AGENTCTL_LINUX_BINARY: agentctlLinuxBinary,
|
||||
KANDEV_MOCK_AGENT_LINUX_BINARY: mockAgentLinuxBinary,
|
||||
}
|
||||
: {}),
|
||||
KANDEV_WORKTREE_ENABLED: "true",
|
||||
KANDEV_WORKTREE_BASEPATH: worktreeBase,
|
||||
KANDEV_REPOCLONE_BASEPATH: repoCloneBase,
|
||||
KANDEV_LOG_LEVEL: process.env.KANDEV_LOG_LEVEL ?? "warn",
|
||||
AGENTCTL_INSTANCE_PORT_BASE: String(agentctlPortBase),
|
||||
AGENTCTL_INSTANCE_PORT_MAX: String(agentctlPortMax),
|
||||
// AGENTCTL_AUTO_APPROVE_PERMISSIONS=true and
|
||||
// KANDEV_PLAN_COALESCE_WINDOW_MS=2000 are applied by the
|
||||
// backend's profile loader (profiles.yaml `e2e:` column).
|
||||
// Specs that need different values (e.g.
|
||||
// permission-approval.spec.ts setting auto-approve=false) set
|
||||
// process.env.X before spawn — that already flows through the
|
||||
// `...sanitizeInheritedEnv(process.env)` spread above, and the
|
||||
// backend's ApplyProfile leaves already-set vars alone. (Note:
|
||||
// KANDEV_FEATURES_* is the exception — it's stripped from the
|
||||
// inherited env so the profile always governs feature flags.)
|
||||
GIT_AUTHOR_NAME: "E2E Test",
|
||||
GIT_AUTHOR_EMAIL: "e2e@test.local",
|
||||
GIT_COMMITTER_NAME: "E2E Test",
|
||||
GIT_COMMITTER_EMAIL: "e2e@test.local",
|
||||
};
|
||||
|
||||
const debug = !!process.env.E2E_DEBUG;
|
||||
const baseUrl = `http://localhost:${backendPort}`;
|
||||
const debug = !!process.env.E2E_DEBUG;
|
||||
const baseUrl = `http://localhost:${backendPort}`;
|
||||
|
||||
// Snapshot the baseline env so `restart(envOverrides)` rebuilds from
|
||||
// a clean copy each call instead of accumulating leftover keys (e.g.
|
||||
// KANDEV_MOCK_PROVIDERS, KANDEV_PROVIDER_FAILURES) from prior tests.
|
||||
const baselineEnv = { ...backendEnv } as Record<string, string>;
|
||||
// Snapshot the baseline env so `restart(envOverrides)` rebuilds from
|
||||
// a clean copy each call instead of accumulating leftover keys (e.g.
|
||||
// KANDEV_MOCK_PROVIDERS, KANDEV_PROVIDER_FAILURES) from prior tests.
|
||||
const baselineEnv = { ...backendEnv } as Record<string, string>;
|
||||
|
||||
// --- Spawn backend ---
|
||||
let backendProc = spawnBackendProcess(backendEnv, debug, backendPort);
|
||||
await waitForHealth(`${baseUrl}/health`, HEALTH_TIMEOUT_MS);
|
||||
const frontendUrl = baseUrl;
|
||||
|
||||
/**
|
||||
* Kill the backend process group and respawn with the same config.
|
||||
* SQLite DB, tmpDir, and all persisted data survive the restart.
|
||||
* Only in-memory execution state (running agents, WS connections) is lost.
|
||||
*/
|
||||
const restart = async (envOverrides?: Record<string, string>) => {
|
||||
// Rebuild from the baseline snapshot so a previous restart's
|
||||
// overrides don't leak into this one (e.g. KANDEV_MOCK_PROVIDERS
|
||||
// set by the routing specs would otherwise stick for the rest of
|
||||
// the worker's lifetime and register canonical agent IDs that
|
||||
// sibling specs count).
|
||||
const nextEnv: Record<string, string> = { ...baselineEnv };
|
||||
if (envOverrides) {
|
||||
for (const [k, v] of Object.entries(envOverrides)) {
|
||||
nextEnv[k] = v;
|
||||
}
|
||||
}
|
||||
await killProcessGroup(backendProc);
|
||||
// Poll until the OS releases the TCP port rather than sleeping a fixed
|
||||
// 2 s. TIME_WAIT can linger for 30–120 s under load; the probe exits
|
||||
// as soon as the port stops accepting connections (typically <200 ms).
|
||||
await waitForPortFree(backendPort);
|
||||
backendProc = spawnBackendProcess(nextEnv, debug, backendPort);
|
||||
// Pass the process so waitForHealth fails fast if it exits (e.g. port still in use)
|
||||
// --- Spawn backend ---
|
||||
backendProc = spawnBackendProcess(backendEnv, debug, backendPort);
|
||||
registerProcess(backendProc);
|
||||
await waitForHealth(`${baseUrl}/health`, HEALTH_TIMEOUT_MS, backendProc);
|
||||
};
|
||||
const frontendUrl = baseUrl;
|
||||
|
||||
/**
|
||||
* Kill the backend process group and respawn with the same config.
|
||||
* SQLite DB, tmpDir, and all persisted data survive the restart.
|
||||
* Only in-memory execution state (running agents, WS connections) is lost.
|
||||
*/
|
||||
const restart = async (envOverrides?: Record<string, string>) => {
|
||||
// Rebuild from the baseline snapshot so a previous restart's
|
||||
// overrides don't leak into this one (e.g. KANDEV_MOCK_PROVIDERS
|
||||
// set by the routing specs would otherwise stick for the rest of
|
||||
// the worker's lifetime and register canonical agent IDs that
|
||||
// sibling specs count).
|
||||
const nextEnv: Record<string, string> = { ...baselineEnv };
|
||||
if (envOverrides) {
|
||||
for (const [k, v] of Object.entries(envOverrides)) {
|
||||
nextEnv[k] = v;
|
||||
}
|
||||
}
|
||||
const runningProcess = backendProc;
|
||||
if (!runningProcess) throw new Error("Backend process is not running");
|
||||
await killProcessGroup(runningProcess);
|
||||
// Poll until the OS releases the TCP port rather than sleeping a fixed
|
||||
// 2 s. TIME_WAIT can linger for 30–120 s under load; the probe exits
|
||||
// as soon as the port stops accepting connections (typically <200 ms).
|
||||
await waitForPortFree(backendPort);
|
||||
backendProc = spawnBackendProcess(nextEnv, debug, backendPort);
|
||||
registerProcess(backendProc);
|
||||
// Pass the process so waitForHealth fails fast if it exits (e.g. port still in use)
|
||||
await waitForHealth(`${baseUrl}/health`, HEALTH_TIMEOUT_MS, backendProc);
|
||||
};
|
||||
|
||||
try {
|
||||
await use({ port: backendPort, baseUrl, frontendPort, frontendUrl, tmpDir, restart });
|
||||
} finally {
|
||||
// Shutdown the backend process group.
|
||||
await killProcessGroup(backendProc);
|
||||
|
||||
// Cleanup temp directory — ignore errors (backend may still hold files briefly)
|
||||
try {
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
} catch {
|
||||
// Non-fatal: OS may not have released all file handles yet
|
||||
}
|
||||
}
|
||||
});
|
||||
},
|
||||
{ scope: "worker", timeout: 60_000 },
|
||||
],
|
||||
|
||||
@@ -110,8 +110,26 @@ export function startSSHServer(
|
||||
* Tear down the container started by startSSHServer. Safe to call even if
|
||||
* the container has already exited.
|
||||
*/
|
||||
export function stopSSHServer(handle: SSHServerHandle): void {
|
||||
spawnSync("docker", ["rm", "-f", handle.containerName], { stdio: "ignore" });
|
||||
type DockerCommand = (args: string[]) => void;
|
||||
|
||||
export function stopSSHServer(
|
||||
handle: SSHServerHandle,
|
||||
runDocker: DockerCommand = (args) => {
|
||||
spawnSync("docker", args, { stdio: "ignore" });
|
||||
},
|
||||
): void {
|
||||
// The sshd entrypoint chowns this bind mount to the container user. Hand it
|
||||
// back to the test runner so the backend fixture can remove its exact root.
|
||||
const owner = fs.statSync(handle.workDir);
|
||||
runDocker([
|
||||
"exec",
|
||||
handle.containerName,
|
||||
"chown",
|
||||
"-R",
|
||||
`${owner.uid}:${owner.gid}`,
|
||||
"/home/kandev/.kandev",
|
||||
]);
|
||||
runDocker(["rm", "-f", handle.containerName]);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -0,0 +1,153 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { type ChildProcess } from "node:child_process";
|
||||
import { EventEmitter } from "node:events";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { test } from "@playwright/test";
|
||||
|
||||
import { runOwnedBackendFixture, waitForHealth } from "../../fixtures/backend";
|
||||
import { stopSSHServer, type SSHServerHandle } from "../../helpers/ssh";
|
||||
|
||||
test.describe("backend fixture lifecycle", () => {
|
||||
test("fails fast when the backend exited before health polling began", async () => {
|
||||
const exitedProcess = Object.assign(new EventEmitter(), {
|
||||
exitCode: 17,
|
||||
signalCode: null,
|
||||
}) as unknown as ChildProcess;
|
||||
|
||||
await assert.rejects(
|
||||
waitForHealth("http://127.0.0.1:1/health", 50, exitedProcess),
|
||||
/Backend process exited with code 17/,
|
||||
);
|
||||
});
|
||||
|
||||
test("fails fast when the backend process cannot spawn", async () => {
|
||||
const failedProcess = Object.assign(new EventEmitter(), {
|
||||
exitCode: null,
|
||||
signalCode: null,
|
||||
}) as unknown as ChildProcess;
|
||||
const health = waitForHealth("http://127.0.0.1:1/health", 5_000, failedProcess);
|
||||
|
||||
queueMicrotask(() => failedProcess.emit("error", new Error("spawn failed")));
|
||||
|
||||
await assert.rejects(health, /spawn failed/);
|
||||
});
|
||||
|
||||
test("removes its exact temporary root when setup fails before process spawn", async () => {
|
||||
const parentDir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-lifecycle-test-"));
|
||||
const ownedDir = fs.mkdtempSync(path.join(parentDir, "owned-"));
|
||||
const peerDir = fs.mkdtempSync(path.join(parentDir, "peer-"));
|
||||
fs.writeFileSync(path.join(ownedDir, "partial.db"), "partial");
|
||||
fs.writeFileSync(path.join(peerDir, "keep.db"), "keep");
|
||||
|
||||
try {
|
||||
await assert.rejects(
|
||||
runOwnedBackendFixture(ownedDir, async () => {
|
||||
throw new Error("setup failed");
|
||||
}),
|
||||
/setup failed/,
|
||||
);
|
||||
|
||||
assert.equal(fs.existsSync(ownedDir), false);
|
||||
assert.equal(fs.readFileSync(path.join(peerDir, "keep.db"), "utf8"), "keep");
|
||||
} finally {
|
||||
fs.rmSync(parentDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("removes its temporary root after successful fixture use", async () => {
|
||||
const ownedDir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-lifecycle-success-"));
|
||||
|
||||
await runOwnedBackendFixture(ownedDir, async () => {
|
||||
fs.writeFileSync(path.join(ownedDir, "kandev.db"), "complete");
|
||||
});
|
||||
|
||||
assert.equal(fs.existsSync(ownedDir), false);
|
||||
});
|
||||
|
||||
test("stops the registered process before cleanup after a health failure", async () => {
|
||||
const events: string[] = [];
|
||||
const child = new EventEmitter() as unknown as ChildProcess;
|
||||
|
||||
await assert.rejects(
|
||||
runOwnedBackendFixture(
|
||||
"/tmp/exact-owned-root",
|
||||
async (registerProcess) => {
|
||||
registerProcess(child);
|
||||
events.push("run");
|
||||
throw new Error("health failed");
|
||||
},
|
||||
{
|
||||
stopProcess: async (registeredChild) => {
|
||||
assert.equal(registeredChild, child);
|
||||
events.push("stop");
|
||||
},
|
||||
removeTempRoot: (tmpDir) => {
|
||||
assert.equal(tmpDir, "/tmp/exact-owned-root");
|
||||
events.push("remove");
|
||||
},
|
||||
},
|
||||
),
|
||||
/health failed/,
|
||||
);
|
||||
|
||||
assert.deepEqual(events, ["run", "stop", "remove"]);
|
||||
});
|
||||
|
||||
test("surfaces cleanup failure alongside the fixture failure", async () => {
|
||||
const fixtureFailure = new Error("fixture failed");
|
||||
const cleanupFailure = new Error("cleanup failed");
|
||||
|
||||
await assert.rejects(
|
||||
runOwnedBackendFixture(
|
||||
"/tmp/exact-owned-root",
|
||||
async () => {
|
||||
throw fixtureFailure;
|
||||
},
|
||||
{
|
||||
removeTempRoot: () => {
|
||||
throw cleanupFailure;
|
||||
},
|
||||
},
|
||||
),
|
||||
(error) => {
|
||||
assert.ok(error instanceof AggregateError);
|
||||
assert.equal(error.errors[0], fixtureFailure);
|
||||
assert.match(error.errors[1].message, /Failed to remove E2E temporary root/);
|
||||
assert.equal(error.errors[1].cause, cleanupFailure);
|
||||
return true;
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
test("returns the SSH bind mount to the host owner before removing the container", () => {
|
||||
const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-ssh-cleanup-test-"));
|
||||
const owner = fs.statSync(workDir);
|
||||
const commands: string[][] = [];
|
||||
const handle = {
|
||||
containerName: "kandev-sshd-e2e-7",
|
||||
workDir,
|
||||
} as SSHServerHandle;
|
||||
|
||||
try {
|
||||
stopSSHServer(handle, (args) => {
|
||||
commands.push(args);
|
||||
});
|
||||
} finally {
|
||||
fs.rmSync(workDir, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
assert.deepEqual(commands, [
|
||||
[
|
||||
"exec",
|
||||
handle.containerName,
|
||||
"chown",
|
||||
"-R",
|
||||
`${owner.uid}:${owner.gid}`,
|
||||
"/home/kandev/.kandev",
|
||||
],
|
||||
["rm", "-f", handle.containerName],
|
||||
]);
|
||||
});
|
||||
});
|
||||
@@ -1,6 +1,6 @@
|
||||
# 0045: Install-wide storage maintenance uses typed ownership providers and quarantine
|
||||
|
||||
**Status:** accepted (amended 2026-07-19)
|
||||
**Status:** accepted (amended 2026-07-22)
|
||||
**Date:** 2026-07-14
|
||||
**Area:** backend, frontend, infra
|
||||
|
||||
@@ -44,20 +44,28 @@ pending archive job and restores a matching quarantined workspace when possible.
|
||||
does not delete historical archived-task worktree rows or branch metadata used by PR #1687's branch
|
||||
recovery.
|
||||
|
||||
Agentctl session temporary roots are a separate lifecycle-owned resource. They remain beneath the
|
||||
operating system's temporary directory under a readable, collision-resistant digest of the raw
|
||||
session and instance identity so concurrent sessions have isolated sockets, compiler work files,
|
||||
and command scratch space without mixing ephemeral data into `KANDEV_HOME_DIR`. After agent, shell,
|
||||
VS Code, and workspace subprocesses are reaped during permanent instance teardown, agentctl deletes
|
||||
only that instance's validated owned root. A later session resume creates a replacement instance
|
||||
and does not depend on the prior instance's scratch. Session temp data is not quarantined because it
|
||||
is explicitly ephemeral and contains no task recovery state.
|
||||
Host-local agent instances inherit the Kandev service's `TMPDIR`, `TMP`, and `TEMP` environment
|
||||
unchanged. Agentctl does not replace those values with a per-instance directory and does not claim
|
||||
ownership of files written to the shared operating-system temporary location. An operator-level
|
||||
temporary-directory override remains authoritative for the whole service; with no override, normal
|
||||
platform defaults apply.
|
||||
|
||||
Terminal instance teardown closes process admission before its ownership sweep. A failed HTTP
|
||||
shutdown or process-tree reap retains a stopping instance tombstone and its allocated port so a
|
||||
replacement cannot adopt unresolved runtime resources. A temporary-directory-only deletion failure
|
||||
is reported but does not retain the execution port. Ordinary agent stop/start remains restartable;
|
||||
the irreversible admission close belongs to instance teardown.
|
||||
This boundary is intentionally separate from persistent tool-cache ownership. Go's default
|
||||
`GOCACHE`, for example, is already a shared user cache and is not derived from `TMPDIR`; Kandev only
|
||||
injects its managed cache path when that existing Storage option is explicitly enabled. Tools that
|
||||
derive reusable caches from the system temp directory may now share them across agents instead of
|
||||
duplicating them beneath per-instance roots.
|
||||
|
||||
Terminal instance teardown still closes process admission and reaps the owned process tree, but it
|
||||
does not sweep shared temp contents. Kandev-specific sockets, locks, or scratch paths that require
|
||||
isolation must carry an explicit collision-resistant name at their own boundary. A demonstrated
|
||||
tool-specific collision may justify a narrow override for that tool, not a replacement of the
|
||||
temporary environment inherited by every child process.
|
||||
|
||||
Directories under `/tmp/kandev-agent/*` created by older versions are legacy host data. New agent
|
||||
instances no longer add to that root. Storage maintenance does not delete legacy entries by name or
|
||||
age because the existing directories lack durable ownership evidence; host temporary-file policy or
|
||||
a deliberate stopped-service cleanup remains the authority for that historical footprint.
|
||||
|
||||
## Consequences
|
||||
|
||||
@@ -72,8 +80,13 @@ the irreversible admission close belongs to instance teardown.
|
||||
reclaimable bytes for the operator.
|
||||
- The existing Office GC package and startup wiring are removed or reduced to adapters over the
|
||||
System storage service to avoid two competing sweepers.
|
||||
- Session teardown reclaims its temporary storage immediately and independently of scheduled
|
||||
maintenance, while path containment prevents removal of the shared temp root or sibling sessions.
|
||||
- Node, Playwright, and similar temp-derived caches may be reused across agent instances instead of
|
||||
being duplicated under a task-scoped temp root.
|
||||
- Transient compiler work, test databases, plugin fixtures, sockets, and other scratch still exist;
|
||||
they move to the inherited shared temp location and are governed by tool cleanup plus host policy.
|
||||
- Kandev loses per-task attribution and exact recursive deletion for arbitrary temp contents. This
|
||||
is an accepted tradeoff because operators prefer cache sharing and normal host temp lifecycle.
|
||||
- Existing `/tmp/kandev-agent` usage is not reclaimed automatically by this change.
|
||||
|
||||
## Alternatives Considered
|
||||
|
||||
@@ -93,5 +106,13 @@ the irreversible admission close belongs to instance teardown.
|
||||
- **Store session scratch data under `KANDEV_HOME_DIR`.** Rejected because it mixes transient
|
||||
process data with persistent application state and makes backups, retention, and disk accounting
|
||||
retain data that should disappear at teardown or reboot.
|
||||
- **Share one temporary directory across agent sessions.** Rejected because filenames, sockets, and
|
||||
compiler work directories can collide and teardown cannot safely determine which files it owns.
|
||||
- **Give every agent instance its own `TMPDIR`, `TMP`, and `TEMP`.** Rejected after operational
|
||||
evidence showed duplicated Node/Playwright caches, explicitly redirected Go caches, and abandoned
|
||||
E2E/compiler scratch consuming many GiB beneath long-lived agent roots. It improves attribution
|
||||
and defensive isolation, but prevents useful cache sharing and makes interrupted task roots a
|
||||
concentrated storage leak.
|
||||
- **Keep per-instance temp roots but mount shared subdirectories for selected caches.** Rejected for
|
||||
now because cache discovery is tool-specific and creates a growing allowlist. Persistent caches
|
||||
such as `GOCACHE` retain their dedicated policy; observed collisions can receive narrow fixes.
|
||||
- **Delete old `/tmp/kandev-agent/*` entries by name or age.** Rejected because neither proves task
|
||||
archival, installation ownership, process liveness, or protection from path replacement.
|
||||
|
||||
@@ -52,7 +52,7 @@ Read individual ADRs for full context. Create new ones via `/record decision` or
|
||||
| 2026-07-14-typed-utility-chat-sessions | [Typed Utility Chats Share the Quick Chat Session Model](2026-07-14-typed-utility-chat-sessions.md) | accepted | backend, frontend | 2026-07-14 |
|
||||
| 2026-07-15-office-agent-execution-profile-routing | [Separate Office identity from routed execution profiles](2026-07-15-office-agent-execution-profile-routing.md) | proposed | backend, frontend | 2026-07-15 |
|
||||
| 0044 | [ACP agent compatibility dialects](0044-acp-agent-compatibility-dialects.md) | accepted | backend, protocol | 2026-07-16 |
|
||||
| 0045 | [Install-wide storage maintenance uses typed ownership providers and quarantine](0045-install-wide-storage-maintenance.md) | accepted (amended 2026-07-19) | backend, frontend, infra | 2026-07-14 |
|
||||
| 0045 | [Install-wide storage maintenance uses typed ownership providers and quarantine](0045-install-wide-storage-maintenance.md) | accepted (amended 2026-07-22) | backend, frontend, infra | 2026-07-14 |
|
||||
| 0046 | [Settings route save coordinator](0046-settings-route-save-coordinator.md) | accepted | frontend | 2026-07-14 |
|
||||
| 0047 | [Plugins read conversation content via a capability-gated Host RPC](0047-plugin-host-conversation-reads.md) | accepted | backend, protocol | 2026-07-21 |
|
||||
| 0048 | [Plugins invoke a settings-selectable utility agent](0048-plugin-host-utility-agent-invoke.md) | accepted | backend, frontend, protocol | 2026-07-21 |
|
||||
|
||||
@@ -18,6 +18,12 @@ The implementation preserves PR #1687's unarchive contract: archived filesystem
|
||||
reclaimed, historical worktree rows and branch metadata remain, and pending archive cleanup is
|
||||
cancelled when a task becomes active again.
|
||||
|
||||
The 2026-07-22 follow-up removes per-instance temporary-directory injection. Host-local agents
|
||||
inherit the Kandev service's `TMPDIR`, `TMP`, and `TEMP` unchanged so temp-derived Node/Playwright
|
||||
caches can be reused. The superseded marker, snapshot, Storage-provider, UI, and E2E work is removed;
|
||||
managed Go-cache defaults and injection remain unchanged. A separate test-harness task addresses
|
||||
the observed accumulation of `kandev-e2e-*` roots.
|
||||
|
||||
---
|
||||
|
||||
## Backend
|
||||
@@ -288,3 +294,19 @@ Wave 4:
|
||||
Wave 5:
|
||||
|
||||
- [x] [Task 09 — E2E, QA, and final verification](task-09-e2e-qa-verification.md)
|
||||
|
||||
Wave 6:
|
||||
|
||||
- [x] [Task 10 — Inherit the service temp environment](task-10-inherit-service-temp.md)
|
||||
|
||||
Wave 7:
|
||||
|
||||
- [x] [Task 11 — Contain E2E temporary roots](task-11-e2e-temp-cleanup.md)
|
||||
|
||||
Wave 8:
|
||||
|
||||
- [x] [Task 12 — Update inherited-temp operations docs](task-12-inherited-temp-docs.md)
|
||||
|
||||
Wave 9:
|
||||
|
||||
- [x] [Task 13 — Review and verify inherited temp behavior](task-13-inherited-temp-verification.md)
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
---
|
||||
id: "10-inherit-service-temp"
|
||||
title: "Inherit the service temp environment"
|
||||
status: done
|
||||
wave: 6
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/system-page/storage-maintenance.md"
|
||||
---
|
||||
|
||||
# Task 10: Inherit the service temp environment
|
||||
|
||||
Stop replacing `TMPDIR`, `TMP`, and `TEMP` for host-local agent instances and remove the superseded
|
||||
uncommitted ownership/maintenance implementation.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Agentctl does not create `<system-temp>/kandev-agent/<instance>` or inject per-instance
|
||||
`TMPDIR`, `TMP`, or `TEMP`; configured service values are inherited unchanged and unset values stay
|
||||
unset at Kandev's environment-construction boundary.
|
||||
- Existing agent stop/reap behavior remains intact, but there is no per-instance temp marker,
|
||||
descriptor, cleanup snapshot, Storage provider/API/UI action, or agent-temp E2E scenario in the
|
||||
final diff.
|
||||
- `GOCACHE` behavior is unchanged: the existing managed cache is injected only when enabled, and
|
||||
no new Go cache is placed beneath the inherited temp directory.
|
||||
- Red-first tests prove inherited/set/unset values and prove starting an instance does not create a
|
||||
`kandev-agent` root.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd apps/backend && go test ./internal/agentctl/server/process ./internal/agentctl/server/instance ./internal/agent/runtime/lifecycle ./internal/backendapp ./internal/system/storage/... ./internal/task/service
|
||||
cd apps && pnpm --filter @kandev/web test -- --run components/settings/system/storage hooks/domains/system/use-storage-maintenance.test.tsx
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/agentctl/server/process/manager.go`
|
||||
- `apps/backend/internal/agentctl/server/process/manager_temp_test.go`
|
||||
- superseded agent-temp DTO, lifecycle, task cleanup, Storage provider, frontend, and E2E files in
|
||||
the current uncommitted diff
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Inputs
|
||||
|
||||
- Spec: Agent session temporary data and scenarios
|
||||
- ADR 0045 inherited-temp amendment
|
||||
- Current `git diff`, whose marker/provider/UI changes are superseded rather than additive
|
||||
|
||||
## Output contract
|
||||
|
||||
Report the exact inherited environment behavior, removed superseded surfaces, red/green tests,
|
||||
commands/results, blockers, and update this task plus the serialized plan checkbox when complete.
|
||||
@@ -0,0 +1,63 @@
|
||||
---
|
||||
id: "11-e2e-temp-cleanup"
|
||||
title: "Contain E2E temporary roots"
|
||||
status: complete
|
||||
wave: 7
|
||||
depends_on: ["10-inherit-service-temp"]
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/system-page/storage-maintenance.md"
|
||||
---
|
||||
|
||||
# Task 11: Contain E2E temporary roots
|
||||
|
||||
Prevent normal managed E2E runs from leaving large numbers of `kandev-e2e-*` backend roots in the
|
||||
service's inherited temporary directory.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Backend fixture directories created by one managed run have one explicit run owner and are removed
|
||||
on normal success and failure without deleting another concurrent run's files.
|
||||
- The cleanup guard begins immediately after `mkdtempSync`, covers setup/spawn/initial health
|
||||
failures, passes the spawned process to the initial health wait for fast early-exit detection, and
|
||||
reports cleanup failures instead of silently discarding them.
|
||||
- Cleanup targets only the exact run-owned root; interrupted-process residue remains subject to host
|
||||
temp policy unless a safe existing runner lifecycle can prove ownership.
|
||||
- Tests or a deterministic harness regression prove success/failure cleanup and concurrent-run
|
||||
isolation. No test uses the host's real Kandev data directory.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd apps/web && pnpm e2e:run tests/system/storage-maintenance.spec.ts
|
||||
cd apps/web && pnpm e2e:run tests/system/mobile-storage-maintenance.spec.ts -- --project=mobile-chrome
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/web/e2e/fixtures/backend.ts`
|
||||
- a focused fixture-lifecycle test beside the existing E2E harness tests
|
||||
- focused fixture tests
|
||||
|
||||
## Dependencies
|
||||
|
||||
Task 10.
|
||||
|
||||
## Inputs
|
||||
|
||||
- Read-only audit of 1,099 abandoned `kandev-e2e-*` roots
|
||||
- Existing managed runner cleanup and concurrency conventions
|
||||
|
||||
## Output contract
|
||||
|
||||
Report root ownership, success/failure/interruption semantics, concurrency safety, tests/commands,
|
||||
remaining host-policy boundary, and update this task plus the serialized plan checkbox when done.
|
||||
|
||||
## Completion
|
||||
|
||||
- The atomically-created worker root is now guarded immediately after creation. Setup, spawn,
|
||||
initial-health, test-use, and restart failures all converge on the same exact-root cleanup path.
|
||||
- The latest registered backend process group is stopped before bounded directory removal. Cleanup
|
||||
failures are surfaced, including alongside the original fixture failure.
|
||||
- Focused lifecycle coverage proves early child-exit detection, success/failure cleanup, process-stop
|
||||
ordering, peer-root isolation, and cleanup-error reporting.
|
||||
- Both required managed Storage E2E commands passed, and neither left a new `kandev-e2e-*` root.
|
||||
@@ -0,0 +1,61 @@
|
||||
---
|
||||
id: "12-inherited-temp-docs"
|
||||
title: "Update inherited-temp operations docs"
|
||||
status: complete
|
||||
wave: 8
|
||||
depends_on: ["10-inherit-service-temp", "11-e2e-temp-cleanup"]
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/system-page/storage-maintenance.md"
|
||||
---
|
||||
|
||||
# Task 12: Update inherited-temp operations docs
|
||||
|
||||
Make public operations guidance match the service-inherited temp boundary and legacy cleanup limits.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Public docs explain that host-local agents inherit service `TMPDIR`/`TMP`/`TEMP`, persistent
|
||||
`GOCACHE` is separate, and host temp policy owns shared scratch cleanup.
|
||||
- Docs do not advertise an agent-temp Storage row/action or automatic deletion of legacy
|
||||
`/tmp/kandev-agent/*`; they explain that confirmed-inactive legacy data needs deliberate host
|
||||
cleanup.
|
||||
- Public docs validation passes.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
node --test scripts/validate-public-docs.test.mjs
|
||||
node scripts/validate-public-docs.mjs
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `docs/public/operations.md`
|
||||
- affected spec/ADR/plan wording discovered by docs audit
|
||||
|
||||
## Dependencies
|
||||
|
||||
Tasks 10 and 11.
|
||||
|
||||
## Inputs
|
||||
|
||||
- `/docs-maintainer`
|
||||
- Updated Storage spec and ADR 0045
|
||||
|
||||
## Output contract
|
||||
|
||||
Report public/internal docs updated, validation results, stale terminology removed, blockers, and
|
||||
update this task plus the serialized plan checkbox when complete.
|
||||
|
||||
## Completion
|
||||
|
||||
- Updated the public operations guide to document service-inherited `TMPDIR`, `TMP`, and `TEMP`,
|
||||
platform defaults when those variables are unset, and shared cache behavior for temp-derived
|
||||
tooling.
|
||||
- Clarified that the default and managed Go build caches are governed by `GOCACHE`, with Kandev's
|
||||
managed path remaining opt-in and independent from temporary-directory policy.
|
||||
- Removed the superseded agent-temp Storage analysis/action, marker, cleanup-record, and
|
||||
archive/delete directory-removal guidance. Shared scratch cleanup now belongs to host temp policy.
|
||||
- Documented deliberate host-administrator cleanup for confirmed-inactive legacy
|
||||
`/tmp/kandev-agent/*` directories, with no automatic name- or age-based removal.
|
||||
- Public-doc tests and validation passed; `git diff --check` reported no whitespace errors.
|
||||
@@ -0,0 +1,79 @@
|
||||
---
|
||||
id: "13-inherited-temp-verification"
|
||||
title: "Review and verify inherited temp behavior"
|
||||
status: done
|
||||
wave: 9
|
||||
depends_on: ["10-inherit-service-temp", "11-e2e-temp-cleanup", "12-inherited-temp-docs"]
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/system-page/storage-maintenance.md"
|
||||
---
|
||||
|
||||
# Task 13: Review and verify inherited temp behavior
|
||||
|
||||
Review the simplified implementation for environment, process-lifecycle, cache, concurrency, and
|
||||
documentation regressions, then run repository-wide verification.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Review proves no application path recreates per-instance `kandev-agent` temp roots or injects
|
||||
instance-specific temp variables, while service overrides and managed `GOCACHE` remain correct.
|
||||
- QA covers concurrent agents sharing temp-derived caches, archive/delete process reaping, E2E
|
||||
fixture cleanup, and absence of the superseded Storage API/UI surface.
|
||||
- Formatting, backend tests/lint, frontend typecheck/tests/lint, focused desktop/mobile E2E, public
|
||||
docs validation, and `git diff --check` pass or exact environmental blockers are reported.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
make -C apps/backend fmt
|
||||
make -C apps/backend test
|
||||
make -C apps/backend lint
|
||||
cd apps/web && pnpm run typecheck
|
||||
cd apps && pnpm --filter @kandev/web test
|
||||
cd apps && pnpm --filter @kandev/web lint
|
||||
cd apps/web && pnpm e2e:run tests/system/storage-maintenance.spec.ts
|
||||
cd apps/web && pnpm e2e:run tests/system/mobile-storage-maintenance.spec.ts -- --project=mobile-chrome
|
||||
node --test scripts/validate-public-docs.test.mjs
|
||||
node scripts/validate-public-docs.mjs
|
||||
git diff --check
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- only files required by review, simplification, QA, verification, and docs reconciliation findings
|
||||
|
||||
## Dependencies
|
||||
|
||||
Tasks 10–12.
|
||||
|
||||
## Inputs
|
||||
|
||||
- Completed implementation packets
|
||||
- `/code-review`, `/simplify`, `/qa`, `/verify`, and ADR 0045
|
||||
|
||||
## Output contract
|
||||
|
||||
Report findings/fixes, commands/results, residual collision/legacy-cleanup risks, final spec/plan
|
||||
status, and update this task plus the serialized plan checkbox when complete.
|
||||
|
||||
## Completion
|
||||
|
||||
- Removed the obsolete instance-resource release classification that existed only for the retired
|
||||
agent-temp cleanup error. Every unresolved process teardown now retains the instance and port for
|
||||
retry; focused tests and the process/instance race suites pass.
|
||||
- Removed the stale VS Code temp-setup failure test and reconciled the earlier workspace Git-status
|
||||
plan so it records that per-instance temp teardown was superseded by ADR 0045's amendment.
|
||||
- Hardened backend fixture health polling for asynchronous child-process spawn errors. The fixture
|
||||
lifecycle suite passes all six setup, spawn, health, success, ordering, isolation, and cleanup
|
||||
error cases.
|
||||
- Confirmed production searches contain no agent-temp root creator, temp-variable injector,
|
||||
cleanup classifier, Storage resource/API/UI symbol, or marker implementation. Managed `GOCACHE`
|
||||
injection remains confined to its existing opt-in lifecycle provider paths.
|
||||
- Formatting, repository-wide typecheck/tests/lint, desktop Storage E2E (3 tests), mobile Storage
|
||||
E2E (1 test), public-doc tests (58 tests), public-doc validation (40 pages), race tests, and
|
||||
`git diff --check` pass. Both managed E2E runs left the pre-existing `/tmp/kandev-e2e-*` count
|
||||
unchanged at three.
|
||||
- Residual boundaries are deliberate: shared-temp tool collisions require narrow tool-specific
|
||||
fixes when observed; explicit Unix service temp paths must remain short enough for local socket
|
||||
limits; an uncatchable hard kill can still leave E2E scratch for host policy; and inactive legacy
|
||||
`/tmp/kandev-agent/*` roots require deliberate stopped-service cleanup.
|
||||
@@ -10,6 +10,10 @@ status: done
|
||||
|
||||
First keep reusable verification caches shared and all verification storage outside Git worktrees, ignore legacy worktree-local cache paths, and reclaim each owned agent temp root during permanent instance teardown. Then make diff enrichment linear and context-aware while preserving complete changed-path metadata and all existing limits. Next coalesce overlapping live observations through the workspace tracker using Kandev's established detached, bounded singleflight pattern. Finish with HTTP-boundary concurrency and multi-repository regression coverage.
|
||||
|
||||
The agent-temp teardown portion is a historical implementation record. ADR 0045 was amended on
|
||||
2026-07-22, and Storage Maintenance Task 10 superseded it by removing per-instance temp injection
|
||||
and cleanup. Host-local agents now inherit the Kandev service temporary environment.
|
||||
|
||||
The incident that motivated this work involved 13,869 untracked Go cache files. Diff enrichment repeatedly recomputed total diff bytes by scanning the complete file map inside per-file loops, producing quadratic work. Six concurrent fresh multi-repository requests then repeated that computation and held agentctl near full CPU for roughly three minutes.
|
||||
|
||||
## Backend
|
||||
@@ -105,7 +109,8 @@ Wave 0:
|
||||
Wave 1 (parallel):
|
||||
|
||||
- [x] [task-01-linear-diff-enrichment](task-01-linear-diff-enrichment.md) - done
|
||||
- [x] [task-04-agent-temp-teardown](task-04-agent-temp-teardown.md) - done
|
||||
- [x] [task-04-agent-temp-teardown](task-04-agent-temp-teardown.md) - superseded by Storage
|
||||
Maintenance Task 10
|
||||
|
||||
Wave 2 (depends on Wave 1):
|
||||
|
||||
|
||||
@@ -1,15 +1,20 @@
|
||||
---
|
||||
id: "04-agent-temp-teardown"
|
||||
title: "Agent temp teardown"
|
||||
status: done
|
||||
status: superseded
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/system-page/storage-maintenance.md"
|
||||
superseded_by: "../storage-maintenance/task-10-inherit-service-temp.md"
|
||||
---
|
||||
|
||||
# Task 04: Agent temp teardown
|
||||
|
||||
> Superseded on 2026-07-22 by the inherited service-temp boundary in ADR 0045 and Storage
|
||||
> Maintenance Task 10. This file records the earlier implementation packet; the per-instance
|
||||
> directory and cleanup behavior below is no longer active.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Agent instance teardown removes its exact collision-resistant `<system-temp>/kandev-agent/<readable-prefix>-<identity-digest>` directory only after owned subprocesses are stopped and reaped.
|
||||
|
||||
@@ -86,6 +86,26 @@ build-cache and unused-image cleanup remain disabled until you confirm that Kand
|
||||
Docker daemon.
|
||||
Do not enable those rules on a daemon shared with unrelated workloads.
|
||||
|
||||
Host-local agents inherit the Kandev service's `TMPDIR`, `TMP`, and `TEMP` values unchanged. If the
|
||||
service leaves them unset, agent tools use their normal operating-system defaults; if an operator
|
||||
sets them on the service, all host-local agents share those configured locations. This permits
|
||||
tools whose caches are derived from the temporary directory, such as Node or Playwright tooling, to
|
||||
reuse cache data across agent instances. Kandev does not create a per-instance temporary root or
|
||||
claim ownership of arbitrary files in the shared temporary directory. On Unix, keep an explicit
|
||||
service temp path short enough for local Unix-domain socket paths; an excessively long path can
|
||||
exceed the platform socket-address limit.
|
||||
|
||||
Persistent Go build caching is separate: Go's default `GOCACHE` is not derived from `TMPDIR`, and
|
||||
Kandev injects its managed Go-cache location only when that opt-in Storage setting is enabled.
|
||||
Scratch cleanup in the inherited temporary directory belongs to the operating system or the host's
|
||||
temporary-file policy. Archive and delete stop and reap the task's host-local processes, but do not
|
||||
recursively delete shared temporary files.
|
||||
|
||||
Older versions may have left inactive directories under `/tmp/kandev-agent/*`. Storage analysis and
|
||||
cleanup do not delete this legacy data by name or age. Remove it only through a deliberate
|
||||
host-administrator procedure after confirming that no live process references the target; stopping
|
||||
the Kandev service first provides the clearest maintenance boundary.
|
||||
|
||||
## Database operation
|
||||
|
||||
### SQLite
|
||||
@@ -196,7 +216,7 @@ When reporting an incident, record timestamp/timezone, Kandev version and commit
|
||||
|
||||
**Settings > System > Status** walks `data`, worktrees, repositories, sessions, tasks, quick chat, and backups. Results are cached for two hours; **Refresh** forces a new single-flight walk. Permission failures appear as warnings. The displayed total intentionally counts `data/backups` both inside the `data` row and again as the separate `backups` row, so use filesystem or volume metrics for quota enforcement.
|
||||
|
||||
Archiving or deleting a task stops active sessions and starts asynchronous cleanup with a 60-second bound. Depending on executor, cleanup can delete a managed worktree and its local branch, remove a container, destroy a Sprite, or attempt to stop the remote SSH controller and remove only its per-session runtime directory. SSH process/session cleanup is best-effort when the connection is failing, and the task directory always remains for deliberate, audited cleanup; there is no automatic sweeper for it today. The task can disappear from the UI before cleanup finishes.
|
||||
Archiving or deleting a task stops active sessions and starts durable asynchronous cleanup. Depending on executor, cleanup can delete a managed worktree and its local branch, remove a container, destroy a Sprite, reap a host-local agent's process tree, or attempt to stop the remote SSH controller and remove only its per-session runtime directory. Failed cleanup remains retryable across a backend restart. Kandev does not sweep arbitrary files from the shared temporary directory during archive or delete. SSH process/session cleanup is best-effort when the connection is failing, and the task directory always remains for deliberate, audited cleanup; there is no automatic sweeper for it today. The task can disappear from the UI before cleanup finishes.
|
||||
|
||||
**Reset Environment** uses a separate teardown path. For Sprites, the current reset request can lose the profile credential context and report success while leaving the provider sandbox behind. After a Sprites reset, inspect **Settings > Executors > Sprites.dev** and explicitly destroy the old sandbox there if it remains. See [Executors](executors.md#spritesdev) for the executor-specific lifecycle.
|
||||
|
||||
@@ -261,6 +281,7 @@ drawer mirrors it as the saved left sequence followed by the saved right sequenc
|
||||
| **Apply update** is absent | Install method shown on the Updates page | Expected for current native services; upgrade the package, reinstall the service with the same flags, and restart |
|
||||
| Metrics show unavailable | OS support, disk path, executor connectivity | Select supported metrics and verify permissions/network; the collector reports errors per sample |
|
||||
| Disk total exceeds filesystem expectation | Separate `data` and `backups` rows | Backups are counted twice in the UI total; use volume metrics for capacity decisions |
|
||||
| Legacy `/tmp/kandev-agent/*` uses disk | Process inventory and open-file references for the exact directory | This is data from older Kandev versions, not a current Storage resource. Stop Kandev, confirm no live process references the target, then remove only the confirmed-inactive legacy directory through host administration. |
|
||||
| Archived task's remote resource remains | Backend, Docker/SSH/Sprites, and provider logs | Cleanup is asynchronous and bounded. SSH task directories are retained by design; for other leftovers, verify work is preserved, then remove the exact resource manually |
|
||||
| Sprites reset removed the environment but not the sandbox | **Settings > Executors > Sprites.dev** | Current reset can omit the provider credential during destroy; find the old Kandev-named sandbox and destroy it explicitly |
|
||||
|
||||
|
||||
@@ -140,26 +140,26 @@ Decision: [ADR-2026-07-19-workspace-symlink-entries](../../decisions/2026-07-19-
|
||||
|
||||
### Agent session temporary data
|
||||
|
||||
- Each agent instance receives an isolated operating-system temporary directory at
|
||||
`<system-temp>/kandev-agent/<readable-prefix>-<identity-digest>` through `TMPDIR`, `TMP`, and
|
||||
`TEMP`. The deterministic name includes the raw session ID, instance ID, and port so distinct
|
||||
instance identities cannot collide after unsafe characters are sanitized.
|
||||
- The directory is owned by that agent instance and remains available while its agent, shell,
|
||||
VS Code, or workspace subprocesses may still be running.
|
||||
- Instance teardown first closes every process-start admission path, including requests already in
|
||||
flight, then removes the owned session directory only after each process leader and its owned
|
||||
process tree are reaped. On Unix this includes verified process-group disappearance; Windows uses
|
||||
the executor's existing process-tree termination primitive.
|
||||
Teardown validates that the target is a non-root child of the Kandev agent-temp root and never
|
||||
removes the shared root or a sibling session directory.
|
||||
- Cleanup runs whenever the owning agentctl instance is permanently deleted, including task/session
|
||||
archive and delete stops and when teardown observes that the main agent process has already
|
||||
stopped. A later resume creates a new instance and does not depend on the prior instance's scratch
|
||||
files. A cleanup failure is reported without broadening deletion to another path. The instance
|
||||
and port remain reserved only when HTTP shutdown or process reaping is unresolved; a
|
||||
temporary-directory-only failure retains a retry tombstone but releases the execution port.
|
||||
- Session temporary data is ephemeral and is not quarantined, restored, or included in task
|
||||
recovery. See [ADR 0045](../../decisions/0045-install-wide-storage-maintenance.md).
|
||||
- Host-local agent instances inherit `TMPDIR`, `TMP`, and `TEMP` from the Kandev service unchanged.
|
||||
Kandev does not create or inject a per-instance temporary root. When the service leaves those
|
||||
variables unset, agents and their child tools use the operating system default temporary
|
||||
location; when an operator configures them for the service, every host-local agent shares that
|
||||
configured location.
|
||||
- Tool-managed caches may therefore be shared when the tool's own default uses the temporary
|
||||
location. Persistent caches remain governed by their own variables and policies: in particular,
|
||||
Go's default `GOCACHE` is separate from `TMPDIR`, and Kandev only injects its managed Go-cache path
|
||||
when the existing Storage setting is explicitly enabled.
|
||||
- Kandev-specific files that require collision-free identity must use an explicit unique path or
|
||||
filename. A future collision in one tool is fixed at that tool boundary; it does not justify
|
||||
replacing the complete temporary environment for every agent child process.
|
||||
- Archive/delete teardown still closes process admission and reaps each owned process tree. It does
|
||||
not recursively delete arbitrary files from the inherited system temporary directory because
|
||||
those files are shared and cannot be attributed safely to one task.
|
||||
- Existing `/tmp/kandev-agent/*` directories created by older versions are legacy host data. The
|
||||
Storage scheduler does not delete them by name or age, and new agent runs do not add to that root.
|
||||
Operators may remove confirmed-inactive legacy data through their normal host temporary-file
|
||||
policy or a deliberate one-time maintenance procedure. See
|
||||
[ADR 0045](../../decisions/0045-install-wide-storage-maintenance.md).
|
||||
|
||||
### Docker storage
|
||||
|
||||
@@ -460,6 +460,14 @@ enabling host-global Docker cleanup require explicit UI confirmation and server-
|
||||
- **GIVEN** `/root/.cache/go-build` is the service user's default Go cache and is not adopted,
|
||||
**WHEN** storage analysis runs, **THEN** its path and bytes are reported read-only while cleanup
|
||||
remains unavailable for that path.
|
||||
- **GIVEN** the Kandev service has no temporary-directory variables configured, **WHEN** two
|
||||
host-local agents start, **THEN** neither instance receives an injected `TMPDIR`, `TMP`, or `TEMP`
|
||||
value and their tools use the operating system defaults.
|
||||
- **GIVEN** an operator sets `TMPDIR`, `TMP`, or `TEMP` on the Kandev service, **WHEN** a host-local
|
||||
agent starts, **THEN** it inherits those values unchanged rather than receiving a per-instance
|
||||
replacement.
|
||||
- **GIVEN** a task is archived or deleted, **WHEN** its local instance tears down, **THEN** Kandev
|
||||
reaps its owned processes but does not sweep the shared default temporary directory.
|
||||
- **GIVEN** the Docker daemon reports image-layer usage, **WHEN** storage analysis runs, **THEN**
|
||||
image-layer bytes are shown separately from build-cache and managed-container writable bytes.
|
||||
- **GIVEN** an exited container has `kandev.managed=true` and its task is positively absent,
|
||||
@@ -484,6 +492,10 @@ enabling host-global Docker cleanup require explicit UI confirmation and server-
|
||||
- Cleaning remote SSH executor filesystems; remote maintenance requires a separate explicit design.
|
||||
- Restoring uncommitted files after their quarantine retention has expired.
|
||||
- Automatically cleaning a pre-existing user Go cache without explicit path adoption.
|
||||
- Age-based or name-based deletion of unmarked `/tmp/kandev-agent/*` directories.
|
||||
- A Kandev-owned general-purpose sweeper for the operating system's shared temporary directory.
|
||||
- Guaranteed compatibility with tools that require a fixed, globally unique name in shared temp;
|
||||
those tools need a scoped path override when a real collision is observed.
|
||||
|
||||
## Implementation plan
|
||||
|
||||
|
||||
Reference in New Issue
Block a user