fix: preserve archived task resume state (#3115)
This commit is contained in:
@@ -12,6 +12,7 @@ import (
|
||||
"net"
|
||||
"net/netip"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
@@ -421,6 +422,13 @@ func (c *Client) RemoveContainer(ctx context.Context, containerID string, force
|
||||
// removed the container already, which is the desired end state.
|
||||
return nil
|
||||
}
|
||||
if errdefs.IsConflict(err) && strings.Contains(strings.ToLower(err.Error()), "already in progress") {
|
||||
// Docker rejects a concurrent ContainerRemove with a conflict while
|
||||
// the first removal is still completing. The other owner has already
|
||||
// taken responsibility for this container's teardown, so treat this
|
||||
// response as the same converged end state as a not-found response.
|
||||
return nil
|
||||
}
|
||||
c.logger.Error("Failed to remove container", zap.String("container_id", containerID), zap.Error(err))
|
||||
return fmt.Errorf("failed to remove container %s: %w", containerID, err)
|
||||
}
|
||||
|
||||
@@ -166,3 +166,29 @@ func TestContainerTeardownTreatsMissingContainersAsSuccess(t *testing.T) {
|
||||
require.NoError(t, client.KillContainer(context.Background(), "missing-container", "SIGKILL"))
|
||||
require.NoError(t, client.RemoveContainer(context.Background(), "missing-container", true))
|
||||
}
|
||||
|
||||
func TestRemoveContainerTreatsConcurrentRemovalAsSuccess(t *testing.T) {
|
||||
dockerDaemon := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method == http.MethodHead && r.URL.Path == "/_ping" {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
return
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(http.StatusConflict)
|
||||
_, _ = w.Write([]byte(`{"message":"removal of container abc is already in progress"}`))
|
||||
}))
|
||||
t.Cleanup(dockerDaemon.Close)
|
||||
|
||||
log, err := logger.NewFromZap(zap.NewNop())
|
||||
require.NoError(t, err)
|
||||
client, err := NewClient(config.DockerConfig{
|
||||
Host: "tcp://" + strings.TrimPrefix(dockerDaemon.URL, "http://"),
|
||||
APIVersion: "1.44",
|
||||
}, log)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Docker reports a conflict when another owner is already removing the
|
||||
// same container. Removal is a convergence operation, so the in-flight
|
||||
// owner has already taken responsibility for reaching the desired state.
|
||||
require.NoError(t, client.RemoveContainer(context.Background(), "abc", true))
|
||||
}
|
||||
|
||||
@@ -0,0 +1,277 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/kandev/kandev/internal/task/models"
|
||||
"github.com/kandev/kandev/internal/worktree"
|
||||
)
|
||||
|
||||
type archiveManagerEnvironmentDestroyer struct {
|
||||
mgr *worktree.Manager
|
||||
}
|
||||
|
||||
type policyRecordingWorktreeCleanup struct {
|
||||
worktrees []*worktree.Worktree
|
||||
defaultCalls int
|
||||
preservingCalls int
|
||||
}
|
||||
|
||||
func (*policyRecordingWorktreeCleanup) OnTaskDeleted(context.Context, string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (c *policyRecordingWorktreeCleanup) GetAllByTaskID(context.Context, string) ([]*worktree.Worktree, error) {
|
||||
return c.worktrees, nil
|
||||
}
|
||||
|
||||
func (c *policyRecordingWorktreeCleanup) CleanupWorktrees(context.Context, []*worktree.Worktree) error {
|
||||
c.defaultCalls++
|
||||
return nil
|
||||
}
|
||||
|
||||
func (c *policyRecordingWorktreeCleanup) CleanupWorktreesPreservingBranches(context.Context, []*worktree.Worktree) error {
|
||||
c.preservingCalls++
|
||||
return nil
|
||||
}
|
||||
|
||||
func (*archiveManagerEnvironmentDestroyer) DestroyContainer(context.Context, string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (*archiveManagerEnvironmentDestroyer) DestroySandbox(context.Context, string, string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (d *archiveManagerEnvironmentDestroyer) DestroyWorktree(ctx context.Context, worktreeID string) error {
|
||||
return d.mgr.RemoveByID(ctx, worktreeID, false)
|
||||
}
|
||||
|
||||
func (*archiveManagerEnvironmentDestroyer) PushEnvironmentBranch(context.Context, *models.TaskEnvironment) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (*archiveManagerEnvironmentDestroyer) GetContainerLiveStatus(context.Context, string) (*ContainerLiveStatus, error) {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// TestArchiveTaskCleanupPreservesTaskEnvironmentIdentity covers
|
||||
// AC-TASKS-RUNTIME-CLEANUP-001.6 and AC-TASKS-RUNTIME-CLEANUP-001.7.
|
||||
func TestArchiveTaskCleanupPreservesTaskEnvironmentIdentity(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
svc, _, repo := createTestService(t)
|
||||
const (
|
||||
taskID = "task-archive-identity"
|
||||
sessionID = "session-archive-identity"
|
||||
repositoryID = "repo-archive-identity"
|
||||
environmentID = "env-archive-identity"
|
||||
)
|
||||
seedCleanupTaskAndSession(t, repo, taskID, sessionID)
|
||||
|
||||
sourcePath := initSimpleGitRepo(t)
|
||||
if err := repo.CreateRepository(ctx, &models.Repository{
|
||||
ID: repositoryID, WorkspaceID: "ws-" + taskID, Name: repositoryID,
|
||||
SourceType: "local", LocalPath: sourcePath,
|
||||
}); err != nil {
|
||||
t.Fatalf("CreateRepository: %v", err)
|
||||
}
|
||||
|
||||
mgr := newCleanupTestWorktreeManager(t, repo)
|
||||
wt, err := mgr.Create(ctx, worktree.CreateRequest{
|
||||
TaskID: taskID, SessionID: sessionID, TaskTitle: "Archive identity",
|
||||
RepositoryID: repositoryID, RepositoryPath: sourcePath,
|
||||
BaseBranch: "main", TaskDirName: taskID, RepoName: repositoryID,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("Create worktree: %v", err)
|
||||
}
|
||||
if err := repo.CreateTaskEnvironment(ctx, &models.TaskEnvironment{
|
||||
ID: environmentID, TaskID: taskID, ExecutorType: "worktree",
|
||||
WorkspacePath: filepath.Dir(wt.Path), Status: models.TaskEnvironmentStatusReady,
|
||||
Repos: []*models.TaskEnvironmentRepo{{
|
||||
ID: "env-repo-archive-identity", RepositoryID: repositoryID,
|
||||
BranchSlug: wt.BranchSlug, WorktreeID: wt.ID, WorktreePath: wt.Path,
|
||||
WorktreeBranch: wt.Branch, Status: "active",
|
||||
}},
|
||||
}); err != nil {
|
||||
t.Fatalf("CreateTaskEnvironment: %v", err)
|
||||
}
|
||||
session, err := repo.GetTaskSession(ctx, sessionID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskSession: %v", err)
|
||||
}
|
||||
session.TaskEnvironmentID = environmentID
|
||||
if err := repo.UpdateTaskSession(ctx, session); err != nil {
|
||||
t.Fatalf("UpdateTaskSession: %v", err)
|
||||
}
|
||||
|
||||
svc.SetWorktreeCleanup(mgr)
|
||||
svc.SetEnvironmentDestroyer(&archiveManagerEnvironmentDestroyer{mgr: mgr})
|
||||
svc.setCleanupDoneForTestHook(make(chan struct{}, 1))
|
||||
if err := svc.ArchiveTask(ctx, taskID); err != nil {
|
||||
t.Fatalf("ArchiveTask: %v", err)
|
||||
}
|
||||
waitForCleanupDone(t, svc)
|
||||
|
||||
var cleanupState models.TaskResourceCleanupState
|
||||
if err := repo.DB().QueryRowContext(ctx, `
|
||||
SELECT state FROM task_resource_cleanup_jobs
|
||||
WHERE task_id = ? AND trigger = 'archive'
|
||||
ORDER BY created_at DESC LIMIT 1
|
||||
`, taskID).Scan(&cleanupState); err != nil {
|
||||
t.Fatalf("load archive cleanup state: %v", err)
|
||||
}
|
||||
if cleanupState != models.TaskResourceCleanupStateSucceeded {
|
||||
t.Fatalf("cleanup state = %q, want %q", cleanupState, models.TaskResourceCleanupStateSucceeded)
|
||||
}
|
||||
|
||||
env, err := repo.GetTaskEnvironment(ctx, environmentID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskEnvironment after archive: %v", err)
|
||||
}
|
||||
if env.TaskID != taskID || len(env.Repos) != 1 {
|
||||
t.Fatalf("environment after archive = %+v, want retained owner and repository row", env)
|
||||
}
|
||||
gotRepo := env.Repos[0]
|
||||
if gotRepo.WorktreeID != wt.ID || gotRepo.WorktreePath != wt.Path ||
|
||||
gotRepo.WorktreeBranch != wt.Branch || gotRepo.BranchSlug != wt.BranchSlug {
|
||||
t.Fatalf("repository identity after archive = %+v, want worktree %+v", gotRepo, wt)
|
||||
}
|
||||
if gotRepo.DeletedAt == nil {
|
||||
t.Fatal("repository row remains active after physical archive cleanup")
|
||||
}
|
||||
}
|
||||
|
||||
// TestArchiveUnarchiveResumeReactivatesLocalOnlyBranch covers
|
||||
// AC-TASKS-RUNTIME-CLEANUP-001.7 at the service, SQLite, and Git boundaries.
|
||||
func TestArchiveUnarchiveResumeReactivatesLocalOnlyBranch(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
svc, _, repo := createTestService(t)
|
||||
const (
|
||||
taskID = "task-archive-resume"
|
||||
sessionID = "session-archive-resume"
|
||||
repositoryID = "repo-archive-resume"
|
||||
environmentID = "env-archive-resume"
|
||||
)
|
||||
seedCleanupTaskAndSession(t, repo, taskID, sessionID)
|
||||
|
||||
sourcePath := initSimpleGitRepo(t)
|
||||
if err := repo.CreateRepository(ctx, &models.Repository{
|
||||
ID: repositoryID, WorkspaceID: "ws-" + taskID, Name: repositoryID,
|
||||
SourceType: "local", LocalPath: sourcePath,
|
||||
}); err != nil {
|
||||
t.Fatalf("CreateRepository: %v", err)
|
||||
}
|
||||
mgr := newCleanupTestWorktreeManager(t, repo)
|
||||
request := worktree.CreateRequest{
|
||||
TaskID: taskID, SessionID: sessionID, TaskTitle: "Archive resume",
|
||||
RepositoryID: repositoryID, RepositoryPath: sourcePath,
|
||||
BaseBranch: "main", TaskDirName: taskID, RepoName: repositoryID,
|
||||
}
|
||||
wt, err := mgr.Create(ctx, request)
|
||||
if err != nil {
|
||||
t.Fatalf("Create worktree: %v", err)
|
||||
}
|
||||
runGitTestCmd(t, wt.Path, "commit", "--allow-empty", "-m", "local-only archive work")
|
||||
wantHead := strings.TrimSpace(string(runGitTestCmd(t, wt.Path, "rev-parse", "HEAD")))
|
||||
|
||||
if err := repo.CreateTaskEnvironment(ctx, &models.TaskEnvironment{
|
||||
ID: environmentID, TaskID: taskID, ExecutorType: "worktree",
|
||||
WorkspacePath: filepath.Dir(wt.Path), Status: models.TaskEnvironmentStatusReady,
|
||||
Repos: []*models.TaskEnvironmentRepo{{
|
||||
ID: "env-repo-archive-resume", RepositoryID: repositoryID,
|
||||
BranchSlug: wt.BranchSlug, WorktreeID: wt.ID, WorktreePath: wt.Path,
|
||||
WorktreeBranch: wt.Branch, Status: "active",
|
||||
}},
|
||||
}); err != nil {
|
||||
t.Fatalf("CreateTaskEnvironment: %v", err)
|
||||
}
|
||||
session, err := repo.GetTaskSession(ctx, sessionID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskSession: %v", err)
|
||||
}
|
||||
session.TaskEnvironmentID = environmentID
|
||||
if err := repo.UpdateTaskSession(ctx, session); err != nil {
|
||||
t.Fatalf("UpdateTaskSession: %v", err)
|
||||
}
|
||||
|
||||
svc.SetWorktreeCleanup(mgr)
|
||||
svc.SetEnvironmentDestroyer(&archiveManagerEnvironmentDestroyer{mgr: mgr})
|
||||
svc.setCleanupDoneForTestHook(make(chan struct{}, 1))
|
||||
if err := svc.ArchiveTask(ctx, taskID); err != nil {
|
||||
t.Fatalf("ArchiveTask: %v", err)
|
||||
}
|
||||
waitForCleanupDone(t, svc)
|
||||
|
||||
if got := strings.TrimSpace(string(runGitTestCmd(t, sourcePath, "branch", "--list", wt.Branch))); got == "" {
|
||||
t.Fatalf("archive cleanup deleted local-only branch %q", wt.Branch)
|
||||
}
|
||||
if unarchived, err := repo.UnarchiveTask(ctx, taskID); err != nil || !unarchived {
|
||||
t.Fatalf("UnarchiveTask = %t, %v", unarchived, err)
|
||||
}
|
||||
|
||||
request.WorktreeID = wt.ID
|
||||
restored, err := mgr.Create(ctx, request)
|
||||
if err != nil {
|
||||
t.Fatalf("resume worktree after unarchive: %v", err)
|
||||
}
|
||||
if restored.ID != wt.ID || restored.Path != wt.Path {
|
||||
t.Fatalf("restored worktree = %+v, want ID %q and path %q", restored, wt.ID, wt.Path)
|
||||
}
|
||||
if got := strings.TrimSpace(string(runGitTestCmd(t, restored.Path, "rev-parse", "HEAD"))); got != wantHead {
|
||||
t.Fatalf("restored HEAD = %q, want %q", got, wantHead)
|
||||
}
|
||||
env, err := repo.GetTaskEnvironment(ctx, environmentID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskEnvironment after resume: %v", err)
|
||||
}
|
||||
if len(env.Repos) != 1 || env.Repos[0].WorktreeID != wt.ID || env.Repos[0].DeletedAt != nil {
|
||||
t.Fatalf("environment repository after resume = %+v, want reactivated worktree %q", env.Repos, wt.ID)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCleanupTaskResources_CascadeArchivePreservesBranches(t *testing.T) {
|
||||
svc, _, repo := createTestService(t)
|
||||
const taskID = "task-cascade-archive-policy"
|
||||
seedCleanupTaskAndSession(t, repo, taskID, "session-cascade-archive-policy")
|
||||
cleanup := &policyRecordingWorktreeCleanup{worktrees: []*worktree.Worktree{{
|
||||
ID: "worktree-cascade-archive-policy", TaskID: taskID,
|
||||
}}}
|
||||
svc.SetWorktreeCleanup(cleanup)
|
||||
svc.setCleanupDoneForTestHook(make(chan struct{}, 1))
|
||||
|
||||
svc.CleanupTaskResources(context.Background(), taskID, false)
|
||||
waitForCleanupDone(t, svc)
|
||||
|
||||
if cleanup.preservingCalls != 1 || cleanup.defaultCalls != 0 {
|
||||
t.Fatalf("cleanup calls = preserving %d, default %d; want preserving 1, default 0",
|
||||
cleanup.preservingCalls, cleanup.defaultCalls)
|
||||
}
|
||||
}
|
||||
|
||||
// This contract test covers the retry-safe fallback after the preserving
|
||||
// manager path is in place: archive cleanup must never call a branch-deleting
|
||||
// compatibility method.
|
||||
func TestArchiveCleanupFailsClosedWithoutBranchPreservingCleaner(t *testing.T) {
|
||||
svc, _, _ := createTestService(t)
|
||||
cleanup := &recordingWorktreeCleanup{}
|
||||
svc.SetWorktreeCleanup(cleanup)
|
||||
|
||||
errs := svc.cleanupDestructiveTaskResources(
|
||||
context.Background(), "task-archive-fail-closed", nil,
|
||||
[]*worktree.Worktree{{ID: "worktree-archive-fail-closed", TaskID: "task-archive-fail-closed"}},
|
||||
taskEnvironmentCleanup{preserveBranches: true}, nil,
|
||||
)
|
||||
|
||||
joined := errors.Join(errs...)
|
||||
if joined == nil || !strings.Contains(joined.Error(), "cannot preserve branches") {
|
||||
t.Fatalf("archive cleanup error = %v, want branch-preservation failure", joined)
|
||||
}
|
||||
if got := cleanup.cleanedIDs(); len(got) != 0 {
|
||||
t.Fatalf("branch-deleting cleanup calls = %v, want none", got)
|
||||
}
|
||||
}
|
||||
@@ -343,6 +343,12 @@ func (s *Service) processTaskResourceCleanupJob(ctx context.Context, id string)
|
||||
if err := json.Unmarshal([]byte(job.ResourceSnapshot), &snapshot); err != nil {
|
||||
return s.retryTaskResourceCleanupJob(runCtx, job, fmt.Errorf("decode resource snapshot: %w", err))
|
||||
}
|
||||
// Archive snapshots written by older versions can request environment-row
|
||||
// deletion. The lifecycle trigger is authoritative, so normalize that
|
||||
// stale flag before any destructive step and persist the corrected snapshot.
|
||||
if job.IsArchive() {
|
||||
snapshot.DeleteEnvironmentRow = false
|
||||
}
|
||||
defer s.signalCleanupDoneForTest()
|
||||
cleanupErr := s.executeTaskResourceCleanupJob(runCtx, job, &snapshot)
|
||||
if cleanupErr != nil {
|
||||
@@ -440,12 +446,15 @@ func (s *Service) executeTaskResourceCleanupJob(
|
||||
return err
|
||||
}
|
||||
errs := s.performTaskCleanup(ctx, job.TaskID, snapshot.Sessions, snapshot.Worktrees, targets,
|
||||
taskEnvironmentCleanup{env: snapshot.TaskEnvironment, deleteRow: snapshot.DeleteEnvironmentRow},
|
||||
taskEnvironmentCleanup{
|
||||
env: snapshot.TaskEnvironment, deleteRow: snapshot.DeleteEnvironmentRow,
|
||||
preserveBranches: job.IsArchive(),
|
||||
},
|
||||
taskCleanupPreserveRows(stopOutcome))
|
||||
if cause := context.Cause(ctx); cause != nil {
|
||||
return errors.Join(append(errs, cause)...)
|
||||
}
|
||||
if snapshot.LegacyWorktreeCleanup && len(failedStops) == 0 && s.worktreeCleanup != nil {
|
||||
if snapshot.LegacyWorktreeCleanup && !job.IsArchive() && len(failedStops) == 0 && s.worktreeCleanup != nil {
|
||||
if err := s.worktreeCleanup.OnTaskDeleted(ctx, job.TaskID); err != nil {
|
||||
errs = append(errs, fmt.Errorf("legacy worktree cleanup: %w", err))
|
||||
}
|
||||
|
||||
@@ -55,6 +55,12 @@ func (b *joinCleanupBarrier) CleanupWorktrees(ctx context.Context, _ []*worktree
|
||||
return ctx.Err()
|
||||
}
|
||||
|
||||
func (b *joinCleanupBarrier) CleanupWorktreesPreservingBranches(
|
||||
ctx context.Context, worktrees []*worktree.Worktree,
|
||||
) error {
|
||||
return b.CleanupWorktrees(ctx, worktrees)
|
||||
}
|
||||
|
||||
type recordingLegacyCleanup struct {
|
||||
calls int
|
||||
}
|
||||
@@ -222,6 +228,12 @@ func (b *activityCleanupBarrier) CleanupWorktrees(ctx context.Context, _ []*work
|
||||
}
|
||||
}
|
||||
|
||||
func (b *activityCleanupBarrier) CleanupWorktreesPreservingBranches(
|
||||
ctx context.Context, worktrees []*worktree.Worktree,
|
||||
) error {
|
||||
return b.CleanupWorktrees(ctx, worktrees)
|
||||
}
|
||||
|
||||
func TestClaimedCleanupDrainsMaintenanceAndHoldsActivityThroughDestructiveWork(t *testing.T) {
|
||||
taskSvc, repo := setupOfficeTest(t)
|
||||
coordinator := activity.NewCoordinator(activity.Options{})
|
||||
@@ -308,6 +320,12 @@ func (c *cancellableCleanupBarrier) CleanupWorktrees(ctx context.Context, _ []*w
|
||||
}
|
||||
}
|
||||
|
||||
func (c *cancellableCleanupBarrier) CleanupWorktreesPreservingBranches(
|
||||
ctx context.Context, worktrees []*worktree.Worktree,
|
||||
) error {
|
||||
return c.CleanupWorktrees(ctx, worktrees)
|
||||
}
|
||||
|
||||
func TestUnarchiveCancelsAndJoinsClaimedArchiveCleanup(t *testing.T) {
|
||||
taskSvc, repo := setupOfficeTest(t)
|
||||
ctx := context.Background()
|
||||
@@ -489,6 +507,85 @@ func TestUnarchiveCancellationPreservesCleanupResourcesAfterBlockedCleaner(t *te
|
||||
}
|
||||
}
|
||||
|
||||
func TestArchiveCleanupNormalizesLegacyEnvironmentDeletionFlag(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
svc, _, repo := createTestService(t)
|
||||
const taskID = "task-legacy-archive-snapshot"
|
||||
seedCleanupTaskAndSession(t, repo, taskID, "session-legacy-archive-snapshot")
|
||||
env := &models.TaskEnvironment{
|
||||
ID: "env-legacy-archive-snapshot", TaskID: taskID,
|
||||
ExecutorType: "worktree", ContainerID: "legacy-container",
|
||||
WorkspacePath: "/tmp/legacy-archive", Status: models.TaskEnvironmentStatusReady,
|
||||
Repos: []*models.TaskEnvironmentRepo{{
|
||||
ID: "env-repo-legacy-archive", RepositoryID: "repo-legacy-archive",
|
||||
WorktreeID: "worktree-legacy-archive", WorktreePath: "/tmp/legacy-archive/repo",
|
||||
WorktreeBranch: "feature/legacy-archive", Status: "deleted",
|
||||
}},
|
||||
}
|
||||
if err := repo.CreateTaskEnvironment(ctx, env); err != nil {
|
||||
t.Fatalf("CreateTaskEnvironment: %v", err)
|
||||
}
|
||||
if err := repo.ArchiveTask(ctx, taskID); err != nil {
|
||||
t.Fatalf("ArchiveTask: %v", err)
|
||||
}
|
||||
snapshot, err := json.Marshal(taskResourceCleanupSnapshot{
|
||||
TaskEnvironment: env,
|
||||
DeleteEnvironmentRow: true, // legacy archive snapshot written before the fix
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("marshal legacy snapshot: %v", err)
|
||||
}
|
||||
job := &models.TaskResourceCleanupJob{
|
||||
ID: "archive-legacy-snapshot", OperationID: "archive:" + taskID,
|
||||
TaskID: taskID, Trigger: models.TaskResourceCleanupTriggerArchive,
|
||||
State: models.TaskResourceCleanupStatePending, ResourceSnapshot: string(snapshot),
|
||||
}
|
||||
if err := repo.CreateTaskResourceCleanupJob(ctx, job); err != nil {
|
||||
t.Fatalf("CreateTaskResourceCleanupJob: %v", err)
|
||||
}
|
||||
svc.SetEnvironmentDestroyer(&stubDestroyer{})
|
||||
if err := svc.processTaskResourceCleanupJob(ctx, job.ID); err != nil {
|
||||
t.Fatalf("processTaskResourceCleanupJob: %v", err)
|
||||
}
|
||||
got, err := repo.GetTaskEnvironment(ctx, env.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskEnvironment after legacy archive cleanup: %v", err)
|
||||
}
|
||||
if got.TaskID != taskID || len(got.Repos) != 1 || got.Repos[0].WorktreeID != env.Repos[0].WorktreeID {
|
||||
t.Fatalf("legacy archive environment = %+v, want retained owner and repository identity", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestArchiveCleanupSkipsLegacyDestructiveFallback(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
svc, _, repo := createTestService(t)
|
||||
const taskID = "task-legacy-archive-fallback"
|
||||
seedCleanupTaskAndSession(t, repo, taskID, "session-legacy-archive-fallback")
|
||||
if err := repo.ArchiveTask(ctx, taskID); err != nil {
|
||||
t.Fatalf("ArchiveTask: %v", err)
|
||||
}
|
||||
snapshot, err := json.Marshal(taskResourceCleanupSnapshot{LegacyWorktreeCleanup: true})
|
||||
if err != nil {
|
||||
t.Fatalf("marshal legacy fallback snapshot: %v", err)
|
||||
}
|
||||
job := &models.TaskResourceCleanupJob{
|
||||
ID: "archive-legacy-fallback", OperationID: "archive:" + taskID,
|
||||
TaskID: taskID, Trigger: models.TaskResourceCleanupTriggerArchive,
|
||||
State: models.TaskResourceCleanupStatePending, ResourceSnapshot: string(snapshot),
|
||||
}
|
||||
if err := repo.CreateTaskResourceCleanupJob(ctx, job); err != nil {
|
||||
t.Fatalf("CreateTaskResourceCleanupJob: %v", err)
|
||||
}
|
||||
legacy := &recordingLegacyCleanup{}
|
||||
svc.SetWorktreeCleanup(legacy)
|
||||
if err := svc.processTaskResourceCleanupJob(ctx, job.ID); err != nil {
|
||||
t.Fatalf("processTaskResourceCleanupJob: %v", err)
|
||||
}
|
||||
if legacy.calls != 0 {
|
||||
t.Fatalf("legacy archive cleanup calls = %d, want 0", legacy.calls)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExecuteTaskResourceCleanupJob_CancellationSkipsLegacyCleanup(t *testing.T) {
|
||||
taskSvc, _, _ := createTestService(t)
|
||||
legacy := &recordingLegacyCleanup{}
|
||||
|
||||
@@ -60,6 +60,13 @@ type WorktreeBatchCleaner interface {
|
||||
CleanupWorktrees(ctx context.Context, worktrees []*worktree.Worktree) error
|
||||
}
|
||||
|
||||
// WorktreeArchiveBatchCleaner removes archived task worktrees without deleting
|
||||
// the local branches required for later recovery.
|
||||
type WorktreeArchiveBatchCleaner interface {
|
||||
WorktreeBatchCleaner
|
||||
CleanupWorktreesPreservingBranches(ctx context.Context, worktrees []*worktree.Worktree) error
|
||||
}
|
||||
|
||||
type worktreeReferenceGuard interface {
|
||||
CountActiveWorktreeReferences(ctx context.Context, worktreeID string, excludeSessionIDs []string) (int, error)
|
||||
ReleaseWorktreeReference(ctx context.Context, wt *worktree.Worktree) error
|
||||
|
||||
@@ -85,8 +85,9 @@ type taskStopTarget struct {
|
||||
}
|
||||
|
||||
type taskEnvironmentCleanup struct {
|
||||
env *models.TaskEnvironment
|
||||
deleteRow bool
|
||||
env *models.TaskEnvironment
|
||||
deleteRow bool
|
||||
preserveBranches bool
|
||||
}
|
||||
|
||||
type taskEnvironmentSessionUsageChecker interface {
|
||||
@@ -2041,7 +2042,7 @@ func (s *Service) ArchiveTask(ctx context.Context, id string) error {
|
||||
if err != nil {
|
||||
return fmt.Errorf("lookup task environment for archive: %w", err)
|
||||
}
|
||||
envCleanup := taskEnvironmentCleanup{env: taskEnv, deleteRow: true}
|
||||
envCleanup := taskEnvironmentCleanup{env: taskEnv, deleteRow: false, preserveBranches: true}
|
||||
cleanupJob, err := s.persistTaskResourceCleanup(
|
||||
ctx, id, models.TaskResourceCleanupTriggerArchive, "",
|
||||
sessions, worktrees, stopTargets, envCleanup, true,
|
||||
@@ -2367,6 +2368,9 @@ func (s *Service) deleteTaskStopTargets(ctx context.Context, id string) ([]taskS
|
||||
// for delete cascade, false for archive — archive preserves the row). Runtime
|
||||
// inventory failures abort cleanup so durable stop handles remain retryable.
|
||||
func (s *Service) CleanupTaskResources(ctx context.Context, taskID string, deleteEnvRow bool) {
|
||||
// Capture the lifecycle disposition before borrower ownership transfer can
|
||||
// change whether the environment row itself is deleted.
|
||||
preserveBranches := !deleteEnvRow
|
||||
taskDeleted := deleteEnvRow
|
||||
if deleteEnvRow && s.attachmentSvc != nil {
|
||||
if err := s.attachmentSvc.DeleteByTask(context.WithoutCancel(ctx), taskID); err != nil {
|
||||
@@ -2419,7 +2423,9 @@ func (s *Service) CleanupTaskResources(ctx context.Context, taskID string, delet
|
||||
zap.String("new_owner_task_id", taskEnv.TaskID))
|
||||
}
|
||||
}
|
||||
envCleanup := taskEnvironmentCleanup{env: taskEnv, deleteRow: deleteEnvRow}
|
||||
envCleanup := taskEnvironmentCleanup{
|
||||
env: taskEnv, deleteRow: deleteEnvRow, preserveBranches: preserveBranches,
|
||||
}
|
||||
if len(sessions) == 0 && len(worktrees) == 0 && len(stopTargets) == 0 && taskEnv == nil {
|
||||
return
|
||||
}
|
||||
@@ -3131,18 +3137,24 @@ func (s *Service) cleanupDestructiveTaskResources(
|
||||
}
|
||||
return errs
|
||||
}
|
||||
cleaner, ok := s.worktreeCleanup.(WorktreeBatchCleaner)
|
||||
if !ok {
|
||||
return errs
|
||||
}
|
||||
if cause := context.Cause(ctx); cause != nil {
|
||||
return append(errs, cause)
|
||||
}
|
||||
if err := cleaner.CleanupWorktrees(ctx, worktrees); err != nil {
|
||||
s.logger.Warn("failed to cleanup worktrees after delete",
|
||||
var cleanupErr error
|
||||
if envCleanup.preserveBranches {
|
||||
cleaner, ok := s.worktreeCleanup.(WorktreeArchiveBatchCleaner)
|
||||
if !ok {
|
||||
return append(errs, errors.New("worktree cleaner cannot preserve branches during archive cleanup"))
|
||||
}
|
||||
cleanupErr = cleaner.CleanupWorktreesPreservingBranches(ctx, worktrees)
|
||||
} else if cleaner, ok := s.worktreeCleanup.(WorktreeBatchCleaner); ok {
|
||||
cleanupErr = cleaner.CleanupWorktrees(ctx, worktrees)
|
||||
}
|
||||
if cleanupErr != nil {
|
||||
s.logger.Warn("failed to cleanup task worktrees",
|
||||
zap.String("task_id", taskID),
|
||||
zap.Error(err))
|
||||
errs = append(errs, fmt.Errorf("cleanup worktrees: %w", err))
|
||||
zap.Error(cleanupErr))
|
||||
errs = append(errs, fmt.Errorf("cleanup worktrees: %w", cleanupErr))
|
||||
}
|
||||
return errs
|
||||
}
|
||||
|
||||
@@ -403,6 +403,16 @@ func (m *Manager) managedScriptEnvironment(ctx context.Context) map[string]strin
|
||||
|
||||
// CleanupWorktrees removes provided worktrees without re-fetching from the store.
|
||||
func (m *Manager) CleanupWorktrees(ctx context.Context, worktrees []*Worktree) error {
|
||||
return m.cleanupWorktrees(ctx, worktrees, true)
|
||||
}
|
||||
|
||||
// CleanupWorktreesPreservingBranches removes provided worktrees while retaining
|
||||
// their local branch refs for later archive recovery.
|
||||
func (m *Manager) CleanupWorktreesPreservingBranches(ctx context.Context, worktrees []*Worktree) error {
|
||||
return m.cleanupWorktrees(ctx, worktrees, false)
|
||||
}
|
||||
|
||||
func (m *Manager) cleanupWorktrees(ctx context.Context, worktrees []*Worktree, removeBranch bool) error {
|
||||
if len(worktrees) == 0 {
|
||||
return nil
|
||||
}
|
||||
@@ -418,8 +428,8 @@ func (m *Manager) CleanupWorktrees(ctx context.Context, worktrees []*Worktree) e
|
||||
if strings.TrimSpace(wt.ID) == "" {
|
||||
continue
|
||||
}
|
||||
if err := m.removeWorktree(ctx, wt, true); err != nil {
|
||||
m.logger.Warn("failed to remove worktree on task deletion",
|
||||
if err := m.removeWorktree(ctx, wt, removeBranch); err != nil {
|
||||
m.logger.Warn("failed to remove worktree during batch cleanup",
|
||||
zap.String("task_id", wt.TaskID),
|
||||
zap.String("worktree_id", wt.ID),
|
||||
zap.Error(err))
|
||||
|
||||
@@ -0,0 +1,45 @@
|
||||
package worktree
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/kandev/kandev/internal/task/models"
|
||||
)
|
||||
|
||||
func TestCleanupWorktrees_PreservesBranchWhenRequested(t *testing.T) {
|
||||
mgr, store := newReferenceCleanupTestManager(t)
|
||||
seedReferenceCleanupSession(t, store, "task-archive", "session-archive", models.TaskSessionStateCompleted)
|
||||
wt := createReferenceCleanupWorktree(t, mgr, "task-archive", "session-archive")
|
||||
runGit(t, wt.Path, "commit", "--allow-empty", "-m", "local-only archive work")
|
||||
wantHead := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD"))
|
||||
|
||||
if err := mgr.CleanupWorktreesPreservingBranches(context.Background(), []*Worktree{wt}); err != nil {
|
||||
t.Fatalf("branch-preserving cleanup: %v", err)
|
||||
}
|
||||
|
||||
if _, err := os.Stat(wt.Path); !os.IsNotExist(err) {
|
||||
t.Fatalf("worktree path remains after cleanup, stat error = %v", err)
|
||||
}
|
||||
if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "rev-parse", wt.Branch)); got != wantHead {
|
||||
t.Fatalf("preserved branch head = %q, want %q", got, wantHead)
|
||||
}
|
||||
assertWorktreeReferenceStatus(t, store, wt.ID, StatusDeleted)
|
||||
}
|
||||
|
||||
func TestCleanupWorktrees_RemovesBranchByDefault(t *testing.T) {
|
||||
mgr, store := newReferenceCleanupTestManager(t)
|
||||
seedReferenceCleanupSession(t, store, "task-delete", "session-delete", models.TaskSessionStateCompleted)
|
||||
wt := createReferenceCleanupWorktree(t, mgr, "task-delete", "session-delete")
|
||||
|
||||
if err := mgr.CleanupWorktrees(context.Background(), []*Worktree{wt}); err != nil {
|
||||
t.Fatalf("CleanupWorktrees: %v", err)
|
||||
}
|
||||
|
||||
if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "branch", "--list", wt.Branch)); got != "" {
|
||||
t.Fatalf("default cleanup preserved branch %q", got)
|
||||
}
|
||||
assertWorktreeReferenceStatus(t, store, wt.ID, StatusDeleted)
|
||||
}
|
||||
@@ -0,0 +1,149 @@
|
||||
---
|
||||
created: 2026-08-27
|
||||
status: done
|
||||
requirements:
|
||||
- REQ-TASKS-RUNTIME-CLEANUP-001
|
||||
system_design:
|
||||
- ../../specs/tasks/system-design/runtime-cleanup.md
|
||||
legacy_specs: []
|
||||
---
|
||||
|
||||
# Implementation Plan: Preserve Archive Resume Identity
|
||||
|
||||
## Overview
|
||||
|
||||
Keep an archived task resumable after cleanup removes its worktree directory.
|
||||
Direct archive must retain the task environment owner. Every archive cleanup
|
||||
pass must also retain the local branch ref. Unarchive can then use normal
|
||||
worktree preparation to reactivate the recorded workspace.
|
||||
|
||||
## Confirmed root cause
|
||||
|
||||
The affected task was archived through `Service.ArchiveTask`. Its cleanup
|
||||
snapshot set `delete_environment_row=true`. The successful cleanup job then
|
||||
deleted the `task_environments` row and its repository rows. The session kept a
|
||||
dangling environment ID, so resume could not resolve the workspace owner.
|
||||
|
||||
The cleanup also had two worktree teardown paths. The environment destroyer
|
||||
preserved the local branch. The later batch cleanup removed the same worktree
|
||||
with branch deletion enabled. The hosting provider had already deleted the
|
||||
merged remote branch, so the recorded checkout branch was unavailable locally
|
||||
and remotely.
|
||||
|
||||
Cascade archive already passes `deleteEnvironmentRow=false`. Existing tests
|
||||
cover that path. They did not cover direct archive with a production-like
|
||||
environment destroyer and a successful cleanup job.
|
||||
|
||||
This behavior violates `AC-TASKS-RUNTIME-CLEANUP-001.7`. That criterion already
|
||||
requires an unarchived task to recreate or reactivate its recoverable worktree.
|
||||
No new product requirement is needed.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope
|
||||
|
||||
- Preserve `task_environments` and `task_environment_repos` rows during direct
|
||||
and cascade archive cleanup.
|
||||
- Preserve local Git branch refs during every archive worktree cleanup pass.
|
||||
- Keep physical worktree directory removal and soft deletion unchanged.
|
||||
- Keep delete cleanup behavior unchanged.
|
||||
- Add production-like SQLite and Git regression coverage.
|
||||
- Prove that unarchive resume reactivates the recorded worktree.
|
||||
|
||||
### Out of scope
|
||||
|
||||
- Automatic repair of other damaged task rows.
|
||||
- Pushing branches during archive or unarchive.
|
||||
- Recreating uncommitted files from a Git snapshot.
|
||||
- Frontend changes, schema changes, or public API changes.
|
||||
- Changes to task deletion semantics.
|
||||
|
||||
## Technical approach
|
||||
|
||||
### Retain the archive owner
|
||||
|
||||
Change the direct `Service.ArchiveTask` snapshot to set
|
||||
`DeleteEnvironmentRow=false`. Keep the existing cascade value. Add a service
|
||||
regression that runs the durable cleanup with an environment destroyer wired.
|
||||
The cleanup job must succeed while the environment and repository rows remain.
|
||||
|
||||
The test must not rely on a missing destroyer. That setup makes cleanup retry
|
||||
before row deletion and caused the existing branch-metadata test to pass for the
|
||||
wrong reason.
|
||||
|
||||
### Carry the archive branch policy through batch cleanup
|
||||
|
||||
Add a branch-preserving batch operation to the worktree manager. Keep the
|
||||
existing `CleanupWorktrees` method as the delete-compatible entry point. Both
|
||||
methods delegate to one implementation with an explicit `removeBranch` value.
|
||||
|
||||
The task cleanup service derives branch removal from the lifecycle operation.
|
||||
Archive and cascade archive pass `removeBranch=false`. Delete paths keep their
|
||||
current branch policy. If an archive cleanup implementation cannot preserve
|
||||
branches, fail the cleanup job for retry. Do not fall back to a branch-deleting
|
||||
method.
|
||||
|
||||
This policy applies to the environment teardown and the later batch pass. A
|
||||
duplicate pass can be idempotent, but it cannot strengthen the disposition.
|
||||
|
||||
### Resume proof
|
||||
|
||||
Use a real SQLite repository and worktree manager. Archive a task whose branch
|
||||
exists only as a local ref. Complete cleanup, unarchive the task, and run normal
|
||||
worktree preparation. Assert that the same environment and worktree identity
|
||||
become active again at the preserved commit.
|
||||
|
||||
## Tests
|
||||
|
||||
- `AC-TASKS-RUNTIME-CLEANUP-001.6`: the durable direct-archive snapshot retains
|
||||
the environment owner through cleanup and retry boundaries.
|
||||
- `AC-TASKS-RUNTIME-CLEANUP-001.7`: an unarchived task reactivates the recorded
|
||||
worktree when no remote branch exists.
|
||||
- Existing cascade archive tests remain green and keep
|
||||
`deleteEnvironmentRow=false`.
|
||||
- Existing delete cleanup tests remain green and keep their current branch
|
||||
removal behavior.
|
||||
|
||||
## E2E tests
|
||||
|
||||
No Playwright test is planned. The defect is in backend cleanup persistence and
|
||||
Git materialization. The SQLite and real-Git integration test covers the full
|
||||
failure boundary without browser timing or UI state.
|
||||
|
||||
## Work orders
|
||||
|
||||
- [x] [Task 01: Retain the archive environment owner](task-01-retain-archive-environment.md)
|
||||
- [x] [Task 02: Preserve archive branch refs](task-02-preserve-archive-branch.md)
|
||||
|
||||
## Verification results
|
||||
|
||||
Completed on 2026-08-28:
|
||||
|
||||
```bash
|
||||
rtk go test ./internal/task/service -run 'Archive.*(Environment|Worktree|Branch)' -count=1
|
||||
rtk go test ./internal/worktree -run 'CleanupWorktrees|RestoresReleasedWorktreeAfterArchive|Recreate' -count=1
|
||||
rtk go test -race ./internal/task/service ./internal/worktree ./internal/orchestrator/executor
|
||||
rtk make -C apps/backend lint
|
||||
python3 scripts/lint-spec-files.test.py
|
||||
python3 scripts/lint-spec-files.py --all
|
||||
node --test scripts/validate-public-docs.test.mjs
|
||||
node scripts/validate-public-docs.mjs
|
||||
rtk git diff --check
|
||||
```
|
||||
|
||||
The focused service regressions passed. The related worktree command passed 25
|
||||
tests. The race command passed 2,186 tests across three packages. Backend lint
|
||||
reported zero issues. All 20 specification-linter tests, all 61 public-doc
|
||||
tests, and both document validators passed.
|
||||
|
||||
## Risks
|
||||
|
||||
- A test without the production-like destroyer can hide environment deletion
|
||||
behind a retryable setup error.
|
||||
- A second worktree cleanup pass can delete the branch unless every pass gets
|
||||
the same archive policy.
|
||||
- Trigger inference from `deleteEnvironmentRow` is unsafe. Some delete paths
|
||||
capture rows before task deletion and rely on the snapshot after cascading
|
||||
rows disappear.
|
||||
- The integration test must use a branch with no remote ref. Otherwise remote
|
||||
recovery can hide local branch deletion.
|
||||
@@ -0,0 +1,107 @@
|
||||
---
|
||||
id: "01-retain-archive-environment"
|
||||
title: "Retain the archive environment owner"
|
||||
status: done
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-TASKS-RUNTIME-CLEANUP-001
|
||||
acceptance_criteria:
|
||||
- AC-TASKS-RUNTIME-CLEANUP-001.6
|
||||
- AC-TASKS-RUNTIME-CLEANUP-001.7
|
||||
system_design:
|
||||
- ../../specs/tasks/system-design/runtime-cleanup.md
|
||||
---
|
||||
|
||||
# Task 01: Retain the Archive Environment Owner
|
||||
|
||||
## Summary
|
||||
|
||||
Make direct archive preserve the task environment row, matching cascade
|
||||
archive. Prove the result with a durable cleanup job that can finish
|
||||
successfully.
|
||||
|
||||
## In scope
|
||||
|
||||
- Add a failing direct-archive regression with a production-like environment
|
||||
destroyer.
|
||||
- Change direct archive to store `DeleteEnvironmentRow=false`.
|
||||
- Assert that the cleanup job succeeds.
|
||||
- Assert that `task_environments` and `task_environment_repos` remain.
|
||||
- Keep the worktree repository row soft-deleted with its identity fields.
|
||||
- Keep cascade archive and delete tests green.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Branch-ref retention. Task 02 owns that policy.
|
||||
- Resume routing changes.
|
||||
- Schema or frontend changes.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Direct archive cleanup does not delete the owning environment row.
|
||||
- Its repository rows retain worktree ID, path, branch, and branch slug.
|
||||
- The durable cleanup job reaches `succeeded` with the destroyer wired.
|
||||
- Direct and cascade archive have the same environment-row disposition.
|
||||
- Delete cleanup keeps its current behavior.
|
||||
|
||||
## TDD sequence
|
||||
|
||||
1. Add `TestArchiveTaskCleanupPreservesTaskEnvironmentIdentity` in a dedicated
|
||||
service test file.
|
||||
2. Wire a real SQLite repository and a production-like environment destroyer.
|
||||
3. Confirm the test fails because the environment row is absent.
|
||||
4. Change the direct archive cleanup snapshot.
|
||||
5. Confirm the focused and related cleanup tests pass.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
# From apps/backend:
|
||||
rtk go test ./internal/task/service -run '^TestArchiveTaskCleanupPreservesTaskEnvironmentIdentity$' -count=1
|
||||
rtk go test ./internal/task/service -run 'TestArchive(TaskCleanupPreservesTaskEnvironmentIdentity|CleanupPreservesHistoricalWorktreeBranchMetadata|TaskTree_InvokesResourceCleanerPerTask)$' -count=1
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/task/service/service_tasks.go`
|
||||
- `apps/backend/internal/task/service/resource_cleanup_archive_identity_test.go`
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Risks
|
||||
|
||||
- The test must inspect the cleanup job state. Row survival alone can mean that
|
||||
cleanup failed before it reached the bad deletion.
|
||||
- Keep the archived worktree row as historical inventory. Do not mark it live
|
||||
while its directory is absent.
|
||||
|
||||
## Parallelism
|
||||
|
||||
Sequential.
|
||||
|
||||
## Inputs
|
||||
|
||||
- `REQ-TASKS-RUNTIME-CLEANUP-001`, especially AC `.6` and `.7`.
|
||||
- `docs/specs/tasks/system-design/runtime-cleanup.md`, section
|
||||
`Archive cleanup disposition`.
|
||||
- Direct archive in `service_tasks.go` and cascade archive in
|
||||
`handoff_cascade.go`.
|
||||
- Existing durable cleanup tests in `resource_cleanup_jobs_test.go`.
|
||||
|
||||
## Results
|
||||
|
||||
Implemented direct-archive environment retention and added a production-wired
|
||||
SQLite/worktree regression. The regression failed on the previous behavior
|
||||
because the successful cleanup removed the environment row, then passed after
|
||||
`ArchiveTask` persisted `DeleteEnvironmentRow=false`.
|
||||
|
||||
Verification passed:
|
||||
|
||||
```text
|
||||
rtk go test ./internal/task/service -run '^TestArchiveTaskCleanupPreservesTaskEnvironmentIdentity$' -count=1
|
||||
rtk go test ./internal/task/service -run 'TestArchive(TaskCleanupPreservesTaskEnvironmentIdentity|CleanupPreservesHistoricalWorktreeBranchMetadata|TaskTree_InvokesResourceCleanerPerTask)$' -count=1
|
||||
```
|
||||
@@ -0,0 +1,137 @@
|
||||
---
|
||||
id: "02-preserve-archive-branch"
|
||||
title: "Preserve archive branch refs"
|
||||
status: done
|
||||
wave: 2
|
||||
depends_on:
|
||||
- "01-retain-archive-environment"
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-TASKS-RUNTIME-CLEANUP-001
|
||||
acceptance_criteria:
|
||||
- AC-TASKS-RUNTIME-CLEANUP-001.7
|
||||
system_design:
|
||||
- ../../specs/tasks/system-design/runtime-cleanup.md
|
||||
---
|
||||
|
||||
# Task 02: Preserve Archive Branch Refs
|
||||
|
||||
## Summary
|
||||
|
||||
Carry one branch-preserving policy through every archive worktree cleanup pass.
|
||||
Then prove that unarchive can reactivate a branch that has no remote ref.
|
||||
|
||||
## In scope
|
||||
|
||||
- Add a worktree batch operation with an explicit branch-removal policy.
|
||||
- Keep the existing batch cleanup entry point compatible with delete cleanup.
|
||||
- Make direct and cascade archive use `removeBranch=false` in every pass.
|
||||
- Fail archive cleanup closed when a cleaner cannot preserve branches.
|
||||
- Add real-Git coverage for a local-only branch.
|
||||
- Add an archive, unarchive, and rematerialization integration regression.
|
||||
- Run the related service, worktree, and executor suites with the race detector.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Pushing or recreating remote branches.
|
||||
- Restoring uncommitted files from snapshot data.
|
||||
- New UI recovery actions.
|
||||
- Changing delete cleanup behavior.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Archive removes the physical worktree directory but keeps the local branch
|
||||
ref at the archived head commit.
|
||||
- A duplicate batch cleanup pass preserves that branch.
|
||||
- Cascade archive uses the same policy.
|
||||
- Delete cleanup keeps its current policy.
|
||||
- Unarchive resume reactivates the same environment repository row and worktree
|
||||
identity without a remote branch.
|
||||
- The recreated worktree points at the preserved commit.
|
||||
|
||||
## TDD sequence
|
||||
|
||||
1. Add a failing batch-cleanup test for `removeBranch=false`.
|
||||
2. Add a failing direct-archive integration test with a local-only branch.
|
||||
3. Add the branch-preserving manager entry point and task cleanup wiring.
|
||||
4. Confirm the archive job succeeds and the local branch survives.
|
||||
5. Unarchive and run normal worktree preparation.
|
||||
6. Assert that the recorded worktree becomes active at the same commit.
|
||||
7. Run focused, package, race, lint, and specification checks.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
# From apps/backend:
|
||||
rtk go test ./internal/worktree -run 'TestCleanupWorktrees_(PreservesBranchWhenRequested|RemovesBranchByDefault)$' -count=1
|
||||
rtk go test ./internal/task/service -run '^TestArchiveUnarchiveResumeReactivatesLocalOnlyBranch$' -count=1
|
||||
rtk go test ./internal/worktree -run 'CleanupWorktrees|RestoresReleasedWorktreeAfterArchive|Recreate' -count=1
|
||||
rtk go test -race ./internal/task/service ./internal/worktree ./internal/orchestrator/executor
|
||||
|
||||
# From the repository root:
|
||||
rtk make -C apps/backend lint
|
||||
python3 scripts/lint-spec-files.test.py
|
||||
python3 scripts/lint-spec-files.py --all
|
||||
git diff --check -- docs/specs docs/plans
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/task/service/service.go`
|
||||
- `apps/backend/internal/task/service/service_tasks.go`
|
||||
- `apps/backend/internal/task/service/resource_cleanup_jobs.go`
|
||||
- `apps/backend/internal/task/service/resource_cleanup_archive_identity_test.go`
|
||||
- `apps/backend/internal/worktree/manager_cleanup.go`
|
||||
- `apps/backend/internal/worktree/manager_cleanup_branch_policy_test.go`
|
||||
|
||||
## Dependencies
|
||||
|
||||
- Task 01 preserves the environment and repository identities that resume uses.
|
||||
|
||||
## Risks
|
||||
|
||||
- Do not infer archive policy from row-deletion flags. Use the lifecycle
|
||||
operation or an explicit cleanup disposition.
|
||||
- A compatibility fallback must never call branch-deleting cleanup for an
|
||||
archive.
|
||||
- The end-to-end test needs no remote ref. A remote ref would hide the defect.
|
||||
|
||||
## Parallelism
|
||||
|
||||
Sequential.
|
||||
|
||||
## Inputs
|
||||
|
||||
- `REQ-TASKS-RUNTIME-CLEANUP-001`, especially AC `.7`.
|
||||
- `docs/specs/tasks/system-design/runtime-cleanup.md`, section
|
||||
`Archive cleanup disposition`.
|
||||
- The environment destroyer adapter in `backendapp/worktree.go`.
|
||||
- Batch cleanup in `worktree/manager_cleanup.go`.
|
||||
- Worktree recreate coverage in `manager_recreate_recovery_test.go`.
|
||||
|
||||
## Results
|
||||
|
||||
Added a branch-preserving batch cleanup operation and kept the existing method
|
||||
as the branch-removing delete path. Direct archive, durable cascade archive,
|
||||
and the legacy cascade path now select the preserving operation. Archive
|
||||
cleanup fails for retry when its cleaner cannot preserve branches.
|
||||
|
||||
The real-Git regression failed on the previous behavior because the second
|
||||
cleanup pass deleted the local-only branch. It now archives, unarchives, and
|
||||
reactivates the same environment repository row and worktree ID at the saved
|
||||
commit.
|
||||
|
||||
Verification passed:
|
||||
|
||||
```text
|
||||
rtk go test ./internal/worktree -run 'TestCleanupWorktrees_(PreservesBranchWhenRequested|RemovesBranchByDefault)$' -count=1
|
||||
rtk go test ./internal/task/service -run '^TestArchiveUnarchiveResumeReactivatesLocalOnlyBranch$' -count=1
|
||||
rtk go test ./internal/worktree -run 'CleanupWorktrees|RestoresReleasedWorktreeAfterArchive|Recreate' -count=1
|
||||
rtk go test -race ./internal/task/service ./internal/worktree ./internal/orchestrator/executor
|
||||
rtk make -C apps/backend lint
|
||||
python3 scripts/lint-spec-files.test.py
|
||||
python3 scripts/lint-spec-files.py --all
|
||||
node --test scripts/validate-public-docs.test.mjs
|
||||
node scripts/validate-public-docs.mjs
|
||||
rtk git diff --check
|
||||
```
|
||||
@@ -404,7 +404,7 @@ When reporting an incident, record timestamp/timezone, Kandev version and commit
|
||||
|
||||
**Settings > System > Status** walks `data`, worktrees, repositories, sessions, tasks, quick chat, and the default `data/backups` directory. Results are cached for two hours; **Refresh** forces a new single-flight walk. Permission failures appear as warnings. Backup files outside the resolved home are not included in the total. 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 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.
|
||||
Archiving or deleting a task stops active sessions and starts durable asynchronous cleanup. Archive can remove a managed worktree directory, but it keeps the local task branch and environment identity. Delete can also remove the local task branch. Other cleanup can remove a container, destroy a Sprite, reap a host-local agent process tree, or stop a remote SSH controller. SSH cleanup removes only the 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. The remote SSH task directory remains for deliberate cleanup. 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.
|
||||
|
||||
|
||||
@@ -587,7 +587,7 @@ Archive records the task as archived and removes it from active views immediatel
|
||||
| Executor | Archive cleanup |
|
||||
| ------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| Local | Attempts to stop the agent runtime; leaves the local folder, files, and branch untouched. |
|
||||
| Git worktree | Attempts to remove the Kandev-owned worktree and its local task branch. It does not delete the remote branch, and shared or borrowed worktrees can remain until their last active user is gone. |
|
||||
| Git worktree | Attempts to remove the Kandev-owned worktree directory. It keeps the local task branch and leaves any existing remote branch untouched. Shared or borrowed worktrees can remain until their last active user is gone. |
|
||||
| Local Docker | Attempts to stop and remove the container; the host repository remains. |
|
||||
| Remote Docker | Runtime create and stop are not implemented. This executor is in progress and cannot currently start a task, so it has no supported archive-cleanup flow. |
|
||||
| Sprites | Attempts to destroy the sandbox; if cleanup succeeds, uncommitted sandbox work is lost. |
|
||||
@@ -595,7 +595,7 @@ Archive records the task as archived and removes it from active views immediatel
|
||||
|
||||
The archive confirmation is enabled by default at **Settings → General → Task Actions → Archive Confirmation** under **Confirm before archiving tasks**. If a parent has children, **Also archive _N_ subtasks** is unchecked by default; without it, the children remain active. Task MCP archive/delete operations affect only the selected task and do not offer the cascade checkbox. MCP delete also does not reparent direct children the way the UI's non-cascade delete does; use the UI rather than task MCP to delete a parent that still has children.
|
||||
|
||||
To restore a task, open **List**, enable **Show archived**, and choose unarchive. If the parent was archived with its children, the cascade-owned children are restored with it. For worktree tasks, Kandev probes the newest historical worktree branch for each repository. If that branch still exists locally or on `origin`, Kandev restores it as the checkout branch so the next session can pick it up. Recovery is best-effort and does not rewrite ambiguous multi-row attachments for the same repository. If the branch is missing, the unarchive toast warns that the next session starts fresh from the base branch; work that existed only on the deleted local branch is unrecoverable. Removed worktree directories, containers, and sandboxes are materialized again on a later launch rather than resumed in place.
|
||||
To restore a task, open **List**, enable **Show archived**, and choose unarchive. If the parent was archived with its children, the cascade-owned children are restored with it. For worktree tasks, archive keeps the environment identity and the local branch. The next session recreates the worktree directory from that branch. Recovery is best-effort and does not rewrite ambiguous multi-row attachments for the same repository. If an external action or an older Kandev version removed the branch, Kandev also checks `origin`. If no branch exists, the next session starts from the base branch. Removed worktree directories, containers, and sandboxes are materialized again on a later launch rather than resumed in place.
|
||||
|
||||
Delete is permanent. If **Also delete _N_ subtasks** is left unchecked, direct children become root tasks. If selected, descendants are deleted. The operation cannot be undone, and executor cleanup follows the same asynchronous, best-effort rules as archive.
|
||||
|
||||
@@ -619,7 +619,7 @@ settled task in the still-working state.
|
||||
- **Remote source cannot clone or fetch:** verify provider credentials and access to every repository and base branch.
|
||||
- **Attachment is rejected below the picker limit:** encoded size is subject to the backend's stricter 10 MB item/batch checks.
|
||||
- **Resources remain after archive or delete:** physical cleanup is asynchronous and best-effort. Check for an active task sharing the environment, a failed runtime stop, and server cleanup logs before removing anything manually.
|
||||
- **An unarchived worktree starts fresh:** the prior branch no longer existed locally or on `origin`; any work that was never pushed or otherwise saved cannot be recovered by Kandev.
|
||||
- **An unarchived worktree starts fresh:** an external action or an older Kandev version removed the branch, and no matching branch exists on `origin`.
|
||||
- **A synchronized workflow is read-only:** edit the workflow file in its GitHub source and let sync apply the change.
|
||||
|
||||
Related: [Coordinate work](coordination.md), [Sessions and review](sessions-and-review.md), [Agents and profiles](agents-and-profiles.md), and [Automation and MCP](automation-and-mcp.md).
|
||||
|
||||
@@ -4,7 +4,7 @@ system: tasks
|
||||
requirements:
|
||||
- REQ-TASKS-RUNTIME-CLEANUP-001
|
||||
created: 2026-06-22
|
||||
updated: 2026-08-24
|
||||
updated: 2026-08-27
|
||||
owners:
|
||||
- cfl
|
||||
---
|
||||
@@ -135,6 +135,35 @@ worktrees, or executor rows behind and the machine slowly runs out of memory.
|
||||
worktree by updating any soft-deleted row with the same environment,
|
||||
repository, and branch identity.
|
||||
|
||||
## Archive cleanup disposition
|
||||
|
||||
The cleanup job trigger defines the resource disposition. Direct archive and
|
||||
cascade archive use the same disposition. A caller-specific Boolean must not
|
||||
change that disposition.
|
||||
|
||||
Archive cleanup performs these actions:
|
||||
|
||||
- Stop the task runtimes and remove executor-specific runtime resources.
|
||||
- Remove the physical worktree directory and mark its repository row deleted.
|
||||
- Preserve the owning `task_environments` row.
|
||||
- Preserve every `task_environment_repos` row, including its worktree ID,
|
||||
path, branch, and branch slug.
|
||||
- Preserve the local Git branch ref. This keeps committed work recoverable when
|
||||
a hosting provider deletes the remote branch after merge.
|
||||
|
||||
The worktree can appear in both the environment snapshot and the batch
|
||||
worktree snapshot. Both cleanup passes must use the archive disposition. A
|
||||
later pass must not delete a branch that an earlier pass preserved.
|
||||
|
||||
Unarchive does not invent a new workspace owner. It keeps the preserved
|
||||
environment link. When the repository row is not live, resume enters normal
|
||||
preparation and reactivates that row. The recreated worktree uses the preserved
|
||||
local branch first, then the existing remote or pull-request recovery paths.
|
||||
|
||||
Delete behavior remains separate. A delete trigger can remove durable owner
|
||||
rows after it captures the cleanup snapshot. This archive rule does not weaken
|
||||
delete cleanup.
|
||||
|
||||
## Data Model
|
||||
|
||||
### `executors_running`
|
||||
@@ -367,6 +396,8 @@ The durable cleanup job wraps that resource lifecycle:
|
||||
not classified as orphaned.
|
||||
- Historical worktree rows for archived tasks remain available to unarchive branch
|
||||
recovery even after their on-disk directories are removed.
|
||||
- Archive cleanup preserves the task environment owner and the local branch ref.
|
||||
Cleanup retries and duplicate teardown passes keep the same disposition.
|
||||
- Orphaned OS processes without any durable `executors_running` row are outside
|
||||
normal cleanup guarantees; they may be handled by an explicit operator recovery
|
||||
tool, but automatic task cleanup must not rely on process-name scanning.
|
||||
@@ -490,6 +521,9 @@ The durable cleanup job wraps that resource lifecycle:
|
||||
- **GIVEN** archive cleanup removed a local worktree, **WHEN** the task is
|
||||
unarchived, **THEN** its historical worktree branch metadata remains available
|
||||
for local/remote recovery.
|
||||
- **GIVEN** a direct or cascade archive removes a task worktree, **WHEN** the
|
||||
remote branch is also absent, **THEN** the task environment, repository row,
|
||||
and local branch ref remain available for normal resume preparation.
|
||||
- **GIVEN** an unarchived task environment whose worktree repository row is
|
||||
deleted, failed, or tombstoned, **WHEN** its session resumes, **THEN** the
|
||||
executor selects normal worktree preparation and recreates or reactivates the
|
||||
@@ -512,3 +546,4 @@ The durable cleanup job wraps that resource lifecycle:
|
||||
|
||||
- [Backend failure containment](../../../plans/backend-failure-containment/plan.md)
|
||||
- [Worktree resume after unarchive](../../../plans/worktree-resume-after-unarchive/plan.md)
|
||||
- [Archive resume identity](../../../plans/archive-resume-identity/plan.md)
|
||||
|
||||
Reference in New Issue
Block a user