diff --git a/apps/backend/internal/agent/runtime/lifecycle/env_preparer.go b/apps/backend/internal/agent/runtime/lifecycle/env_preparer.go index a72bef55d9..9a5487f621 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/env_preparer.go +++ b/apps/backend/internal/agent/runtime/lifecycle/env_preparer.go @@ -73,6 +73,7 @@ type RepoPrepareSpec struct { RepositoryPath string RepoName string BaseBranch string + IntegrationRef string DefaultBranch string // Repository's default_branch, used as fallback when BaseBranch is missing CheckoutBranch string PRNumber int // GitHub PR number when CheckoutBranch is a PR head; enables refs/pull//head fetch for fork PRs. @@ -125,6 +126,7 @@ type EnvPrepareRequest struct { SetupScript string RepoSetupScript string // Repository-level setup script (e.g. "make install") BaseBranch string + IntegrationRef string DefaultBranch string // Repository's default_branch, used as fallback when BaseBranch is missing CheckoutBranch string PRNumber int // GitHub PR number when CheckoutBranch is a PR head; enables refs/pull//head fetch for fork PRs. @@ -185,6 +187,7 @@ func (r *EnvPrepareRequest) RepoSpecs() []RepoPrepareSpec { RepositoryPath: r.RepositoryPath, RepoName: r.RepoName, BaseBranch: r.BaseBranch, + IntegrationRef: r.IntegrationRef, DefaultBranch: r.DefaultBranch, CheckoutBranch: r.CheckoutBranch, PRNumber: r.PRNumber, @@ -230,6 +233,8 @@ type RepoWorktreeResult struct { BranchSlug string `json:"branch_slug,omitempty"` WorktreeID string `json:"worktree_id,omitempty"` WorktreeBranch string `json:"worktree_branch,omitempty"` + WorktreeBranchOwner string `json:"-"` + WorktreeIntegrationRef string `json:"-"` WorktreePath string `json:"worktree_path,omitempty"` MainRepoGitDir string `json:"main_repo_git_dir,omitempty"` RequestedBaseBranch string `json:"requested_base_branch,omitempty"` @@ -250,6 +255,8 @@ type EnvPrepareResult struct { // Legacy single-worktree fields; for multi-repo results they mirror Worktrees[0]. WorktreeID string `json:"worktree_id,omitempty"` WorktreeBranch string `json:"worktree_branch,omitempty"` + WorktreeBranchOwner string `json:"-"` + WorktreeIntegrationRef string `json:"-"` MainRepoGitDir string `json:"main_repo_git_dir,omitempty"` RequestedBaseBranch string `json:"requested_base_branch,omitempty"` BaseBranch string `json:"base_branch,omitempty"` diff --git a/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree.go b/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree.go index 693af1cad0..687063ca30 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree.go +++ b/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree.go @@ -6,11 +6,14 @@ import ( "fmt" "os" "path/filepath" + "strings" "time" "go.uber.org/zap" "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/gitconfigenv" + "github.com/kandev/kandev/internal/githubauth" "github.com/kandev/kandev/internal/worktree" ) @@ -89,13 +92,15 @@ func (p *WorktreePreparer) Prepare(ctx context.Context, req *EnvPrepareRequest, } if req.WorkspaceReuseRequired { return &EnvPrepareResult{ - Success: true, - Steps: steps, - WorkspacePath: wt.Path, - Duration: time.Since(start), - WorktreeID: wt.ID, - WorktreeBranch: wt.Branch, - MainRepoGitDir: filepath.Join(req.RepositoryPath, ".git"), + Success: true, + Steps: steps, + WorkspacePath: wt.Path, + Duration: time.Since(start), + WorktreeID: wt.ID, + WorktreeBranch: wt.Branch, + WorktreeBranchOwner: wt.BranchOwner, + WorktreeIntegrationRef: wt.IntegrationRef, + MainRepoGitDir: filepath.Join(req.RepositoryPath, ".git"), }, nil } @@ -150,6 +155,8 @@ func (p *WorktreePreparer) Prepare(ctx context.Context, req *EnvPrepareRequest, Duration: time.Since(start), WorktreeID: wt.ID, WorktreeBranch: wt.Branch, + WorktreeBranchOwner: wt.BranchOwner, + WorktreeIntegrationRef: wt.IntegrationRef, MainRepoGitDir: mainRepoGitDir, RequestedBaseBranch: req.BaseBranch, BaseBranch: wt.BaseBranch, @@ -293,6 +300,7 @@ func buildWorktreeCreateRequest(req *EnvPrepareRequest) worktree.CreateRequest { RepositoryID: req.RepositoryID, RepositoryPath: req.RepositoryPath, BaseBranch: req.BaseBranch, + IntegrationRef: req.IntegrationRef, FallbackBaseBranch: req.DefaultBranch, CheckoutBranch: req.CheckoutBranch, PRNumber: req.PRNumber, @@ -313,14 +321,53 @@ func buildWorktreeCreateRequest(req *EnvPrepareRequest) worktree.CreateRequest { BranchSlug: req.BranchSlug, BranchIdentitySlug: req.BranchIdentitySlug, ContributionDestination: req.ContributionDestination, - // Export resolved executor-profile env vars into the repository setup - // script so tokens (e.g. an npm auth token) are available during - // install. Set for both single-repo and multi-repo launches, which both - // build their CreateRequest here (multi-repo copies req.Env per spec). - ScriptEnv: req.Env, + // Repository setup scripts receive profile and repository environment, + // but never the managed Git credential broker capabilities used to + // prepare the agent runtime. + ScriptEnv: setupScriptEnvironment(req.Env), } } +func setupScriptEnvironment(env map[string]string) map[string]string { + result := cloneStringMap(env) + for _, key := range append([]string{ + githubauth.CredentialHelperPathEnv, + githubauth.CredentialCLIShimDirEnv, + githubauth.CredentialCLIBashEnvEnv, + githubauth.CredentialParentBashEnv, + }, managedGitCredentialBrokerEnvKeys...) { + delete(result, key) + } + filtered, err := gitconfigenv.Filter(result, func(index int, entries []gitconfigenv.Entry) bool { + return !isManagedSetupScriptGitHelper(index, entries) + }) + if err == nil { + return filtered + } + for key := range result { + if gitconfigenv.IsIndexedKey(key) { + delete(result, key) + } + } + return result +} + +func isManagedSetupScriptGitHelper(index int, entries []gitconfigenv.Entry) bool { + entry := entries[index] + key, value := entry.Key, entry.Value + normalizedKey := strings.ToLower(strings.TrimSpace(key)) + if !strings.HasPrefix(normalizedKey, "credential.https://") || !strings.HasSuffix(normalizedKey, ".helper") { + return false + } + if value == "" && index+1 < len(entries) && entries[index+1].Key == key { + return isManagedSetupScriptGitHelper(index+1, entries) + } + return value == githubauth.ManagedGitCredentialHelper || + value == githubauth.LegacyShimGitCredentialHelper || + value == githubauth.LegacyGitCredentialHelper || + githubauth.IsHostGitHubCredentialHelper(value) +} + // completeCreateWorktreeStep marks the "Create worktree" step successful, // attaches any base-branch fallback warning carried by the worktree, and // reports progress. @@ -445,6 +492,8 @@ func (p *WorktreePreparer) prepareMultiRepo( BranchSlug: repoBranchIdentitySlug(spec), WorktreeID: wt.ID, WorktreeBranch: wt.Branch, + WorktreeBranchOwner: wt.BranchOwner, + WorktreeIntegrationRef: wt.IntegrationRef, WorktreePath: wt.Path, MainRepoGitDir: filepath.Join(spec.RepositoryPath, ".git"), RequestedBaseBranch: spec.BaseBranch, @@ -472,6 +521,8 @@ func (p *WorktreePreparer) prepareMultiRepo( if len(worktrees) > 0 { res.WorktreeID = worktrees[0].WorktreeID res.WorktreeBranch = worktrees[0].WorktreeBranch + res.WorktreeBranchOwner = worktrees[0].WorktreeBranchOwner + res.WorktreeIntegrationRef = worktrees[0].WorktreeIntegrationRef res.MainRepoGitDir = worktrees[0].MainRepoGitDir res.RequestedBaseBranch = worktrees[0].RequestedBaseBranch res.BaseBranch = worktrees[0].BaseBranch @@ -526,6 +577,7 @@ func (p *WorktreePreparer) prepareOneRepo( subReq.RepositoryPath = spec.RepositoryPath subReq.RepoName = spec.RepoName subReq.BaseBranch = spec.BaseBranch + subReq.IntegrationRef = spec.IntegrationRef subReq.DefaultBranch = spec.DefaultBranch subReq.CheckoutBranch = spec.CheckoutBranch subReq.PRNumber = spec.PRNumber diff --git a/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_env_test.go b/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_env_test.go index c7adf2af77..8a45b42d56 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_env_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_env_test.go @@ -1,6 +1,10 @@ package lifecycle -import "testing" +import ( + "testing" + + "github.com/kandev/kandev/internal/githubauth" +) // TestBuildWorktreeCreateRequestForwardsProfileEnv guards that resolved // executor-profile env vars (already merged into EnvPrepareRequest.Env) are @@ -12,6 +16,7 @@ func TestBuildWorktreeCreateRequestForwardsProfileEnv(t *testing.T) { req := &EnvPrepareRequest{ TaskID: "task-1", RepositoryPath: "/repo", + IntegrationRef: "develop", Env: map[string]string{ "FONTAWESOME_NPM_AUTH_TOKEN": "fa-secret-value", }, @@ -22,6 +27,59 @@ func TestBuildWorktreeCreateRequestForwardsProfileEnv(t *testing.T) { if got.ScriptEnv["FONTAWESOME_NPM_AUTH_TOKEN"] != "fa-secret-value" { t.Fatalf("CreateRequest.ScriptEnv = %#v, want profile env forwarded", got.ScriptEnv) } + if got.IntegrationRef != "develop" { + t.Fatalf("CreateRequest.IntegrationRef = %q, want develop", got.IntegrationRef) + } +} + +func TestBuildWorktreeCreateRequestExcludesManagedGitCredentialsFromSetupScript(t *testing.T) { + req := &EnvPrepareRequest{ + Env: map[string]string{ + "PROFILE_NPM_TOKEN": "profile-secret", + githubauth.CredentialBrokerURLEnv: "https://kandev.example/resolve", + githubauth.CredentialLeaseEnv: "opaque-lease", + githubauth.CredentialReissueCapabilityEnv: "reissue-capability", + githubauth.CredentialTaskIDEnv: "task-1", + githubauth.CredentialSessionIDEnv: "session-1", + githubauth.CredentialRepositoryEnv: "repo-1", + githubauth.CredentialHelperPathEnv: "/opt/kandev/agentctl", + "GIT_CONFIG_COUNT": "5", + "GIT_CONFIG_KEY_0": "core.hooksPath", + "GIT_CONFIG_VALUE_0": "/profile/hooks", + "GIT_CONFIG_KEY_1": "credential.https://github.com.helper", + "GIT_CONFIG_VALUE_1": "", + "GIT_CONFIG_KEY_2": "credential.https://github.com.helper", + "GIT_CONFIG_VALUE_2": githubauth.ManagedGitCredentialHelper, + "GIT_CONFIG_KEY_3": "credential.https://gitlab.example.helper", + "GIT_CONFIG_VALUE_3": "!/usr/local/bin/custom-credential-helper", + "GIT_CONFIG_KEY_4": "credential.https://github.com.helper", + "GIT_CONFIG_VALUE_4": "!f() { : " + githubauth.HostGitHubCredentialHelperMarker + "; gh auth git-credential \"$@\"; }; f", + }, + } + + got := buildWorktreeCreateRequest(req).ScriptEnv + + if got["PROFILE_NPM_TOKEN"] != "profile-secret" { + t.Fatalf("profile environment missing from setup script: %#v", got) + } + for _, key := range []string{ + githubauth.CredentialBrokerURLEnv, + githubauth.CredentialLeaseEnv, + githubauth.CredentialReissueCapabilityEnv, + githubauth.CredentialTaskIDEnv, + githubauth.CredentialSessionIDEnv, + githubauth.CredentialRepositoryEnv, + githubauth.CredentialHelperPathEnv, + } { + if _, exists := got[key]; exists { + t.Fatalf("setup script received managed Git credential %q: %#v", key, got) + } + } + if got["GIT_CONFIG_COUNT"] != "2" || + got["GIT_CONFIG_KEY_0"] != "core.hooksPath" || got["GIT_CONFIG_VALUE_0"] != "/profile/hooks" || + got["GIT_CONFIG_KEY_1"] != "credential.https://gitlab.example.helper" || got["GIT_CONFIG_VALUE_1"] != "!/usr/local/bin/custom-credential-helper" { + t.Fatalf("setup script Git config = %#v, want only user-owned entries", got) + } } func TestBuildWorktreeCreateRequestAllowsExplicitBranchReplacement(t *testing.T) { diff --git a/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_multi_test.go b/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_multi_test.go index 13f734a66f..55d2ea9bbe 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_multi_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_multi_test.go @@ -249,6 +249,36 @@ func TestWorktreePreparer_MultiRepo_CreatesWorktreePerRepo(t *testing.T) { } } +func TestWorktreePreparer_SingleRepoProjectsBranchMetadata(t *testing.T) { + repoPath := initBareGitRepo(t, "single-repo-metadata") + preparer, _ := newPreparerForTest(t) + + result, err := preparer.Prepare(context.Background(), &EnvPrepareRequest{ + TaskID: "task-single-metadata", + SessionID: "session-single-metadata", + TaskTitle: "Single repository metadata", + ExecutorType: executor.NameStandalone, + TaskDirName: "single-repo-metadata_aaa", + RepositoryID: "repo-single-metadata", + RepositoryPath: repoPath, + RepoName: "single-repo-metadata", + BaseBranch: "main", + IntegrationRef: "main", + }, nil) + if err != nil { + t.Fatalf("prepare: %v", err) + } + if !result.Success { + t.Fatalf("expected success; steps: %+v err: %s", result.Steps, result.ErrorMessage) + } + if result.WorktreeBranchOwner != worktree.BranchOwnerManaged { + t.Fatalf("single-repo branch owner = %q, want %q", result.WorktreeBranchOwner, worktree.BranchOwnerManaged) + } + if result.WorktreeIntegrationRef != "main" { + t.Fatalf("single-repo integration ref = %q, want main", result.WorktreeIntegrationRef) + } +} + func TestWorktreePreparer_MultiRepo_RollbackOnPartialFailure(t *testing.T) { repoA := initBareGitRepo(t, "good") // repoB is intentionally a non-git directory to force the second create to fail. diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_execution.go b/apps/backend/internal/agent/runtime/lifecycle/manager_execution.go index 7b9865ea73..317b99d64d 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_execution.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_execution.go @@ -1163,6 +1163,7 @@ func (m *Manager) reconcileWorkspaceWorktrees(ctx context.Context, taskID string TaskID: taskID, SessionID: info.SessionID, RepositoryID: repository.RepositoryID, TaskEnvironmentID: info.TaskEnvironmentID, ReuseRequired: info.TaskEnvironmentID != "", RepositoryPath: repository.RepositoryPath, BaseBranch: repository.BaseBranch, + IntegrationRef: repository.IntegrationRef, FallbackBaseBranch: repository.DefaultBranch, CheckoutBranch: repository.CheckoutBranch, WorktreeID: repository.WorktreeID, TaskDirName: info.TaskDirName, WorkspaceID: info.WorkspaceID, RepoName: repository.RepoName, WorktreeBranchPrefix: repository.WorktreeBranchPrefix, diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go b/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go index bfc04ef873..5a600eab14 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go @@ -1210,6 +1210,7 @@ func buildEnvPrepareRequest(req *LaunchRequest, workspacePath string, execName e SetupScript: req.SetupScript, RepoSetupScript: repoSetupScript, BaseBranch: req.BaseBranch, + IntegrationRef: req.IntegrationRef, DefaultBranch: req.DefaultBranch, CheckoutBranch: req.CheckoutBranch, PRNumber: req.PRNumber, @@ -1246,6 +1247,7 @@ func buildEnvPrepareRequest(req *LaunchRequest, workspacePath string, execName e RepositoryPath: r.RepositoryPath, RepoName: r.RepoName, BaseBranch: r.BaseBranch, + IntegrationRef: r.IntegrationRef, DefaultBranch: r.DefaultBranch, CheckoutBranch: r.CheckoutBranch, PRNumber: r.PRNumber, diff --git a/apps/backend/internal/agent/runtime/lifecycle/types.go b/apps/backend/internal/agent/runtime/lifecycle/types.go index 482bcb9dde..0213f690d5 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/types.go +++ b/apps/backend/internal/agent/runtime/lifecycle/types.go @@ -1041,6 +1041,7 @@ type RepoLaunchSpec struct { RepositoryURL string // Clone URL for remote executors that need to clone RepoName string // Repository name used as subdirectory inside TaskDirName BaseBranch string + IntegrationRef string DefaultBranch string // Repository's default_branch, used as fallback when BaseBranch is missing CheckoutBranch string PRNumber int // GitHub PR number when CheckoutBranch is a PR head; enables refs/pull//head fetch for fork PRs. @@ -1087,6 +1088,7 @@ type WorkspaceRepositorySpec struct { RepositoryID string RepositoryPath string RepoName string + IntegrationRef string BaseBranch string DefaultBranch string CheckoutBranch string @@ -1195,6 +1197,7 @@ type LaunchRequest struct { TaskRepositoryID string // Exact task_repositories row for worktree recovery RepositoryPath string // Path to the main repository (for worktree creation) BaseBranch string // Base branch for the worktree (e.g., "main") + IntegrationRef string // Verified terminal integration target for managed branch compaction DefaultBranch string // Repository's default_branch, used as fallback when BaseBranch is missing CheckoutBranch string // Branch to fetch and checkout after worktree creation (e.g., PR head branch) PRNumber int // GitHub PR number when CheckoutBranch is a PR head; enables refs/pull//head fetch for fork PRs. @@ -1249,6 +1252,7 @@ func (r *LaunchRequest) RepoSpecs() []RepoLaunchSpec { RepositoryPath: r.RepositoryPath, RepoName: r.RepoName, BaseBranch: r.BaseBranch, + IntegrationRef: r.IntegrationRef, DefaultBranch: r.DefaultBranch, CheckoutBranch: r.CheckoutBranch, PRNumber: r.PRNumber, diff --git a/apps/backend/internal/agent/runtime/lifecycle/workspace_sources_reconcile.go b/apps/backend/internal/agent/runtime/lifecycle/workspace_sources_reconcile.go index c1744b582e..8b5b83da39 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/workspace_sources_reconcile.go +++ b/apps/backend/internal/agent/runtime/lifecycle/workspace_sources_reconcile.go @@ -158,7 +158,8 @@ func workspaceRepositorySpecsFromLaunch(req *LaunchRequest) []WorkspaceRepositor for _, spec := range specs { result = append(result, WorkspaceRepositorySpec{ RepositoryID: spec.RepositoryID, RepositoryPath: spec.RepositoryPath, RepoName: spec.RepoName, - BaseBranch: spec.BaseBranch, DefaultBranch: spec.DefaultBranch, CheckoutBranch: spec.CheckoutBranch, + IntegrationRef: spec.IntegrationRef, + BaseBranch: spec.BaseBranch, DefaultBranch: spec.DefaultBranch, CheckoutBranch: spec.CheckoutBranch, ComparisonTarget: spec.ComparisonTarget, WorktreeID: spec.WorktreeID, WorktreeBranchPrefix: spec.WorktreeBranchPrefix, WorktreeBranchTemplate: spec.WorktreeBranchTemplate, PullBeforeWorktree: spec.PullBeforeWorktree, diff --git a/apps/backend/internal/backendapp/adapters.go b/apps/backend/internal/backendapp/adapters.go index 4261966b06..2cb1f95ff2 100644 --- a/apps/backend/internal/backendapp/adapters.go +++ b/apps/backend/internal/backendapp/adapters.go @@ -366,6 +366,7 @@ func (a *lifecycleAdapter) LaunchAgent(ctx context.Context, req *executor.Launch // Extract worktree info from metadata if available metadata := execution.MetadataSnapshot() var worktreeID, worktreePath, worktreeBranch string + var worktreeBranchOwner, worktreeIntegrationRef string if metadata != nil { if id, ok := metadata["worktree_id"].(string); ok { worktreeID = id @@ -377,6 +378,10 @@ func (a *lifecycleAdapter) LaunchAgent(ctx context.Context, req *executor.Launch worktreeBranch = branch } } + if execution.PrepareResult != nil { + worktreeBranchOwner = execution.PrepareResult.WorktreeBranchOwner + worktreeIntegrationRef = execution.PrepareResult.WorktreeIntegrationRef + } // Surface per-repo worktree results from the prepare step so the orchestrator // can persist N TaskEnvironmentRepo / TaskSessionWorktree rows when multi-repo. @@ -396,6 +401,8 @@ func (a *lifecycleAdapter) LaunchAgent(ctx context.Context, req *executor.Launch BranchSlug: w.BranchSlug, WorktreeID: w.WorktreeID, WorktreeBranch: w.WorktreeBranch, + WorktreeBranchOwner: w.WorktreeBranchOwner, + WorktreeIntegrationRef: w.WorktreeIntegrationRef, WorktreePath: w.WorktreePath, MainRepoGitDir: w.MainRepoGitDir, RequestedBaseBranch: w.RequestedBaseBranch, @@ -413,6 +420,8 @@ func (a *lifecycleAdapter) LaunchAgent(ctx context.Context, req *executor.Launch WorktreeID: worktreeID, WorktreePath: worktreePath, WorktreeBranch: worktreeBranch, + WorktreeBranchOwner: worktreeBranchOwner, + WorktreeIntegrationRef: worktreeIntegrationRef, RequestedBaseBranch: requestedBaseBranch, BaseBranch: baseBranch, BaseBranchFallbackWarning: baseBranchFallbackWarning, @@ -465,6 +474,7 @@ func buildLifecycleLaunchRequest( TaskRepositoryID: req.TaskRepositoryID, RepositoryPath: req.RepositoryPath, BaseBranch: req.BaseBranch, + IntegrationRef: req.IntegrationRef, DefaultBranch: req.DefaultBranch, CheckoutBranch: req.CheckoutBranch, PRNumber: req.PRNumber, @@ -529,6 +539,7 @@ func lifecycleRepoLaunchSpecs(repos []executor.RepoSpec) []lifecycle.RepoLaunchS RepositoryURL: r.RepositoryURL, RepoName: r.RepoName, BaseBranch: r.BaseBranch, + IntegrationRef: r.IntegrationRef, DefaultBranch: r.DefaultBranch, CheckoutBranch: r.CheckoutBranch, PRNumber: r.PRNumber, diff --git a/apps/backend/internal/backendapp/branch_materializer.go b/apps/backend/internal/backendapp/branch_materializer.go index 67e0b6be54..464a700e91 100644 --- a/apps/backend/internal/backendapp/branch_materializer.go +++ b/apps/backend/internal/backendapp/branch_materializer.go @@ -166,6 +166,7 @@ func (b *branchMaterializer) prepareMaterializeRequest( RepositoryID: repo.ID, RepositoryPath: repo.LocalPath, BaseBranch: tr.BaseBranch, + IntegrationRef: taskRepositoryIntegrationRef(tr), FallbackBaseBranch: repo.DefaultBranch, CheckoutBranch: tr.CheckoutBranch, WorktreeBranchPrefix: repo.WorktreeBranchPrefix, @@ -179,6 +180,16 @@ func (b *branchMaterializer) prepareMaterializeRequest( return req, env, session, slug, true, nil } +func taskRepositoryIntegrationRef(taskRepo *models.TaskRepository) string { + if taskRepo == nil { + return "" + } + if taskRepo.BranchPolicyPullRequestTarget != "" { + return taskRepo.BranchPolicyPullRequestTarget + } + return taskRepo.BaseBranch +} + func taskRepositoryBranchTemplate(repo *models.Repository, taskRepo *models.TaskRepository) string { if taskRepo != nil && taskRepo.BranchPolicyBranchTemplate != "" { return taskRepo.BranchPolicyBranchTemplate diff --git a/apps/backend/internal/backendapp/storage_maintenance.go b/apps/backend/internal/backendapp/storage_maintenance.go index aa5ecc0c0c..9adcb451d8 100644 --- a/apps/backend/internal/backendapp/storage_maintenance.go +++ b/apps/backend/internal/backendapp/storage_maintenance.go @@ -34,7 +34,10 @@ import ( "go.uber.org/zap" ) -const workspaceDependenciesProviderName = "workspace_dependencies" +const ( + workspaceDependenciesProviderName = "workspace_dependencies" + archivedManagedBranchesProviderName = "archived_managed_branches" +) type storageComposition struct { handler *storagepkg.Handler @@ -211,7 +214,7 @@ func prepareStorageDependencies( coordinator: coordinator, goCache: goCache, workspaceFactory: workspaceFactory, cachedOverview: cachedOverview, quarantine: quarantine, - providers: storageCleanupProviders(settings, workspaceFactory, goCache, dockerProvider, quarantine, tempProvider), + providers: storageCleanupProviders(settings, workspaceFactory, goCache, dockerProvider, quarantine, worktreeMgr, tempProvider), }, nil } @@ -283,6 +286,26 @@ type attachmentCleanupProvider struct { service *taskservice.AttachmentService } +type archivedBranchMaintainer interface { + MaintainArchivedBranches(context.Context, int) (worktree.BranchCleanupReceipt, error) +} + +type archivedManagedBranchesCleanupProvider struct { + maintainer archivedBranchMaintainer +} + +func (p archivedManagedBranchesCleanupProvider) Name() string { + return archivedManagedBranchesProviderName +} + +func (p archivedManagedBranchesCleanupProvider) Cleanup(ctx context.Context) (map[string]any, error) { + if p.maintainer == nil { + return nil, nil + } + receipt, err := p.maintainer.MaintainArchivedBranches(ctx, worktree.ArchivedBranchMaintenanceBatchLimit) + return toMap(receipt), err +} + func (p attachmentCleanupProvider) Name() string { return "prompt_attachments" } func (p attachmentCleanupProvider) Cleanup(ctx context.Context) (map[string]any, error) { @@ -844,10 +867,12 @@ func storageCleanupProviders( goCache *gocache.Provider, docker *dockerstore.Provider, quarantine quarantinePurger, + archivedBranches archivedBranchMaintainer, temporary ...storagepkg.CleanupProvider, ) []storagepkg.CleanupProvider { providers := []storagepkg.CleanupProvider{ quarantineCleanupProvider{purger: quarantine}, + archivedManagedBranchesCleanupProvider{maintainer: archivedBranches}, workspaceCleanupAdapter(settings, workspaceFactory), workspaceDependencyCleanupAdapter(settings, workspaceFactory), goCacheCleanupProvider{provider: goCache}, diff --git a/apps/backend/internal/backendapp/storage_maintenance_test.go b/apps/backend/internal/backendapp/storage_maintenance_test.go index b43505fe02..660e89fdcf 100644 --- a/apps/backend/internal/backendapp/storage_maintenance_test.go +++ b/apps/backend/internal/backendapp/storage_maintenance_test.go @@ -25,6 +25,7 @@ import ( "github.com/kandev/kandev/internal/system/storage/gocache" "github.com/kandev/kandev/internal/system/storage/tempstore" "github.com/kandev/kandev/internal/system/storage/workspaces" + "github.com/kandev/kandev/internal/worktree" ) func TestStorageOverviewIncludesQuarantineAndManagedContainers(t *testing.T) { @@ -339,7 +340,7 @@ func TestStorageCleanupProvidersIncludeWorkspaceDependencyCleanup(t *testing.T) workspaceFactory := func(storagepkg.StorageMaintenanceSettings) *workspaces.Provider { return workspaces.New(workspaces.Config{TasksRoot: filepath.Join(home, "tasks"), Store: store}) } - providers := storageCleanupProviders(settings, workspaceFactory, nil, nil, nil) + providers := storageCleanupProviders(settings, workspaceFactory, nil, nil, nil, nil) for _, provider := range providers { if provider.Name() == workspaceDependenciesProviderName { return @@ -348,6 +349,47 @@ func TestStorageCleanupProvidersIncludeWorkspaceDependencyCleanup(t *testing.T) t.Fatalf("storage cleanup providers did not include %s", workspaceDependenciesProviderName) } +func TestStorageCleanupProvidersIncludeArchivedManagedBranches(t *testing.T) { + const providerName = "archived_managed_branches" + providers := storageCleanupProviders(nil, nil, nil, nil, nil, nil) + for _, provider := range providers { + if provider.Name() == providerName { + return + } + } + t.Fatalf("storage cleanup providers did not include %s", providerName) +} + +type recordingArchivedBranchMaintainer struct { + limit int + receipt worktree.BranchCleanupReceipt +} + +func (m *recordingArchivedBranchMaintainer) MaintainArchivedBranches( + _ context.Context, limit int, +) (worktree.BranchCleanupReceipt, error) { + m.limit = limit + return m.receipt, nil +} + +func TestArchivedManagedBranchesProviderUsesBoundedManagerReceipt(t *testing.T) { + maintainer := &recordingArchivedBranchMaintainer{receipt: worktree.BranchCleanupReceipt{ + Attempted: 2, Deleted: 1, Retained: 1, + RetainedReasons: map[worktree.BranchRetentionReason]int{worktree.RetainedNotIntegrated: 1}, + }} + provider := archivedManagedBranchesCleanupProvider{maintainer: maintainer} + result, err := provider.Cleanup(context.Background()) + if err != nil { + t.Fatalf("archived branch cleanup: %v", err) + } + if maintainer.limit != worktree.ArchivedBranchMaintenanceBatchLimit { + t.Fatalf("maintenance limit = %d, want %d", maintainer.limit, worktree.ArchivedBranchMaintenanceBatchLimit) + } + if result["attempted"] != float64(2) || result["deleted"] != float64(1) || result["retained"] != float64(1) { + t.Fatalf("maintenance result = %#v", result) + } +} + func TestWorkspaceDependencyCleanupProviderIsDefaultOff(t *testing.T) { settings, store := newStorageMaintenanceStores(t) home := t.TempDir() @@ -357,7 +399,7 @@ func TestWorkspaceDependencyCleanupProviderIsDefaultOff(t *testing.T) { return workspaces.New(workspaces.Config{TasksRoot: filepath.Join(home, "tasks"), Store: store}) } var dependencyProvider storagepkg.CleanupProvider - for _, provider := range storageCleanupProviders(settings, workspaceFactory, nil, nil, nil) { + for _, provider := range storageCleanupProviders(settings, workspaceFactory, nil, nil, nil, nil) { if provider.Name() == workspaceDependenciesProviderName { dependencyProvider = provider break @@ -900,15 +942,17 @@ func TestQuarantineCleanupProviderPurgesEligibleEntries(t *testing.T) { } func TestStorageCleanupProvidersIncludesQuarantineProvider(t *testing.T) { - providers := storageCleanupProviders(nil, nil, nil, nil, &recordingQuarantinePurger{}) - if len(providers) != 7 { - t.Fatalf("provider count = %d, want 7", len(providers)) + providers := storageCleanupProviders(nil, nil, nil, nil, &recordingQuarantinePurger{}, nil) + if len(providers) != 8 { + t.Fatalf("provider count = %d, want 8", len(providers)) } if providers[0].Name() != "quarantine" { t.Fatalf("first provider = %q, want quarantine", providers[0].Name()) } - if providers[1].Name() != "workspaces" || providers[2].Name() != workspaceDependenciesProviderName || providers[3].Name() != "go_cache" { - t.Fatalf("provider order = %q, %q, %q, want workspaces, workspace_dependencies, go_cache", providers[1].Name(), providers[2].Name(), providers[3].Name()) + if providers[1].Name() != archivedManagedBranchesProviderName || providers[2].Name() != "workspaces" || + providers[3].Name() != workspaceDependenciesProviderName || providers[4].Name() != "go_cache" { + t.Fatalf("provider order = %q, %q, %q, %q, want archived branches, workspaces, workspace_dependencies, go_cache", + providers[1].Name(), providers[2].Name(), providers[3].Name(), providers[4].Name()) } } diff --git a/apps/backend/internal/backendapp/worktree.go b/apps/backend/internal/backendapp/worktree.go index a4b38cb12b..43f2bc32fb 100644 --- a/apps/backend/internal/backendapp/worktree.go +++ b/apps/backend/internal/backendapp/worktree.go @@ -118,8 +118,14 @@ func (a *environmentDestroyerAdapter) DestroySandbox(ctx context.Context, sandbo } func (a *environmentDestroyerAdapter) DestroyWorktree(ctx context.Context, worktreeID string) error { - // removeBranch=false: preserve the branch so unpushed work isn't lost. - return a.worktrees.RemoveByID(ctx, worktreeID, false) + receipt, err := a.worktrees.RemoveByIDWithReceipt(ctx, worktreeID) + if err != nil { + return err + } + if receipt.RetainedReasons[worktree.RetainedActiveReference] > 0 { + return fmt.Errorf("worktree %s is still referenced by another task", worktreeID) + } + return nil } func (a *environmentDestroyerAdapter) GetContainerLiveStatus(ctx context.Context, containerID string) (*taskservice.ContainerLiveStatus, error) { diff --git a/apps/backend/internal/orchestrator/executor/executor.go b/apps/backend/internal/orchestrator/executor/executor.go index 16180f8f36..5b41f99085 100644 --- a/apps/backend/internal/orchestrator/executor/executor.go +++ b/apps/backend/internal/orchestrator/executor/executor.go @@ -545,6 +545,7 @@ type LaunchAgentRequest struct { TaskRepositoryID string // Exact task_repositories row for worktree recovery RepositoryPath string // Path to the main repository (for worktree creation) BaseBranch string // Base branch for the worktree (e.g., "main") + IntegrationRef string // Verified terminal integration target for managed branch compaction DefaultBranch string // Repository's default_branch, used as a fallback when BaseBranch is missing CheckoutBranch string // Branch to fetch and checkout after worktree creation (e.g., PR head branch) PRNumber int // GitHub PR number when CheckoutBranch is a PR head; enables refs/pull//head fetch for fork PRs. @@ -599,6 +600,7 @@ type RepoSpec struct { RepositoryURL string RepoName string BaseBranch string + IntegrationRef string DefaultBranch string // Repository's default_branch, used as fallback when BaseBranch is missing CheckoutBranch string PRNumber int // GitHub PR number when CheckoutBranch is a PR head; enables refs/pull//head fetch for fork PRs. @@ -710,6 +712,8 @@ type LaunchAgentResponse struct { WorktreeID string WorktreePath string WorktreeBranch string + WorktreeBranchOwner string + WorktreeIntegrationRef string RequestedBaseBranch string BaseBranch string BaseBranchFallbackWarning string @@ -731,6 +735,8 @@ type RepoWorktreeResult struct { BranchSlug string WorktreeID string WorktreeBranch string + WorktreeBranchOwner string + WorktreeIntegrationRef string WorktreePath string MainRepoGitDir string RequestedBaseBranch string diff --git a/apps/backend/internal/orchestrator/executor/executor_environment_repo_persistence_test.go b/apps/backend/internal/orchestrator/executor/executor_environment_repo_persistence_test.go index 7a4b197b51..32145ab146 100644 --- a/apps/backend/internal/orchestrator/executor/executor_environment_repo_persistence_test.go +++ b/apps/backend/internal/orchestrator/executor/executor_environment_repo_persistence_test.go @@ -2,7 +2,9 @@ package executor import ( "context" + "strings" "testing" + "time" "github.com/kandev/kandev/internal/task/models" ) @@ -82,3 +84,77 @@ func TestPersistTaskEnvironmentRepos_ReplacesAndDeletesOmittedRows(t *testing.T) t.Fatalf("removed row = %#v, want deleted tombstone", removed) } } + +func TestPersistTaskEnvironmentReposForTransitionClearsCompactionMetadataOnReplacement(t *testing.T) { + repo := newMockRepository() + exec := newTestExecutor(t, &mockAgentManager{}, repo) + const envID = "env-transition-replace-compacted" + compactedAt := time.Now().UTC().Add(-time.Hour) + repo.taskEnvironmentRepos[envID] = []*models.TaskEnvironmentRepo{ + { + ID: "replace", TaskEnvironmentID: envID, RepositoryID: "repo-1", BranchSlug: "main", + WorktreeID: "wt-old", WorktreeBranch: "feature/old", WorktreeBranchOwner: "kandev", + WorktreeIntegrationRef: "main", WorktreeRecoveryHeadSHA: strings.Repeat("a", 40), + WorktreeBranchCompactedAt: &compactedAt, Status: taskEnvironmentRepoStatusDeleted, DeletedAt: &compactedAt, + }, + } + + if err := exec.persistTaskEnvironmentReposForTransition(context.Background(), envID, []*models.TaskEnvironmentRepo{ + { + RepositoryID: "repo-1", BranchSlug: "main", WorktreeID: "wt-new", + WorktreePath: "/new", WorktreeBranch: "feature/new", + WorktreeBranchOwner: "kandev", WorktreeIntegrationRef: "main", + }, + }, true); err != nil { + t.Fatalf("persistTaskEnvironmentReposForTransition: %v", err) + } + + row := repo.taskEnvironmentRepos[envID][0] + if row.Status != taskEnvironmentRepoStatusActive || row.DeletedAt != nil || row.WorktreeID != "wt-new" { + t.Fatalf("replacement row = %#v, want active replacement worktree", row) + } + if row.WorktreeRecoveryHeadSHA != "" || row.WorktreeBranchCompactedAt != nil { + t.Fatalf("replacement row kept stale compaction metadata: head=%q compacted_at=%v", row.WorktreeRecoveryHeadSHA, row.WorktreeBranchCompactedAt) + } +} + +func TestPersistTaskEnvironmentReposForTransitionClearsMissingIntegrationRef(t *testing.T) { + repo := newMockRepository() + exec := newTestExecutor(t, &mockAgentManager{}, repo) + const envID = "env-transition-empty-integration" + repo.taskEnvironmentRepos[envID] = []*models.TaskEnvironmentRepo{ + { + ID: "replace", TaskEnvironmentID: envID, RepositoryID: "repo-1", BranchSlug: "main", + WorktreeID: "wt-old", WorktreeBranch: "feature/old", WorktreeBranchOwner: "kandev", + WorktreeIntegrationRef: "main", Status: taskEnvironmentRepoStatusActive, + }, + } + + if err := exec.persistTaskEnvironmentReposForTransition(context.Background(), envID, []*models.TaskEnvironmentRepo{ + { + RepositoryID: "repo-1", BranchSlug: "main", WorktreeID: "wt-new", + WorktreePath: "/new", WorktreeBranch: "feature/new", + WorktreeBranchOwner: "kandev", WorktreeIntegrationRef: "", + }, + }, true); err != nil { + t.Fatalf("persistTaskEnvironmentReposForTransition: %v", err) + } + + row := repo.taskEnvironmentRepos[envID][0] + if row.WorktreeIntegrationRef != "" { + t.Fatalf("WorktreeIntegrationRef = %q, want empty so cleanup fails closed", row.WorktreeIntegrationRef) + } +} + +func TestEnvironmentReposForLaunch_PersistsBranchCompactionMetadata(t *testing.T) { + repos := environmentReposForLaunch( + &LaunchAgentRequest{}, + &LaunchAgentResponse{Worktrees: []RepoWorktreeResult{{ + RepositoryID: "repo-kandev", WorktreeID: "wt-managed", WorktreeBranch: "feature/task", + WorktreeBranchOwner: "kandev", WorktreeIntegrationRef: "develop", + }}}, + ) + if len(repos) != 1 || repos[0].WorktreeBranchOwner != "kandev" || repos[0].WorktreeIntegrationRef != "develop" { + t.Fatalf("environment repository metadata = %+v", repos) + } +} diff --git a/apps/backend/internal/orchestrator/executor/executor_environment_reuse.go b/apps/backend/internal/orchestrator/executor/executor_environment_reuse.go index fff109cf9c..b0618b938e 100644 --- a/apps/backend/internal/orchestrator/executor/executor_environment_reuse.go +++ b/apps/backend/internal/orchestrator/executor/executor_environment_reuse.go @@ -395,6 +395,7 @@ func topLevelLaunchRepoSpec(req *LaunchAgentRequest) (RepoSpec, bool) { RepositoryURL: req.RepositoryURL, RepoName: req.RepoName, BaseBranch: req.BaseBranch, + IntegrationRef: req.IntegrationRef, DefaultBranch: req.DefaultBranch, CheckoutBranch: req.CheckoutBranch, PRNumber: req.PRNumber, diff --git a/apps/backend/internal/orchestrator/executor/executor_execute.go b/apps/backend/internal/orchestrator/executor/executor_execute.go index 1738f39406..cd1b879349 100644 --- a/apps/backend/internal/orchestrator/executor/executor_execute.go +++ b/apps/backend/internal/orchestrator/executor/executor_execute.go @@ -2193,6 +2193,7 @@ func buildRepoSpecs(allRepos []*repoInfo) []RepoSpec { RepositoryID: info.RepositoryID, RepositoryPath: info.RepositoryPath, BaseBranch: info.BaseBranch, + IntegrationRef: info.IntegrationRef, CheckoutBranch: info.CheckoutBranch, PRNumber: info.PRNumber, RemoteContribution: info.RemoteContribution, @@ -2266,6 +2267,7 @@ func (e *Executor) applyRepositoryConfig(req *LaunchAgentRequest, task *v1.Task, req.TaskRepositoryID = repoInfo.TaskRepositoryID req.RepositoryPath = repoInfo.RepositoryPath req.BaseBranch = repoInfo.BaseBranch + req.IntegrationRef = repoInfo.IntegrationRef req.CheckoutBranch = repoInfo.CheckoutBranch req.PRNumber = repoInfo.PRNumber req.RemoteContribution = repoInfo.RemoteContribution @@ -3020,10 +3022,13 @@ func environmentReposForLaunch(req *LaunchAgentRequest, resp *LaunchAgentRespons branchSlug = topLevelBranchIdentitySlug(req) } worktreeID, worktreePath, worktreeBranch := "", "", "" + worktreeBranchOwner, worktreeIntegrationRef := "", "" if resp.WorktreeID != "" { worktreeID = resp.WorktreeID worktreePath = resp.WorktreePath worktreeBranch = resp.WorktreeBranch + worktreeBranchOwner = resp.WorktreeBranchOwner + worktreeIntegrationRef = resp.WorktreeIntegrationRef } return []*models.TaskEnvironmentRepo{{ RepositoryID: req.RepositoryID, @@ -3032,10 +3037,12 @@ func environmentReposForLaunch(req *LaunchAgentRequest, resp *LaunchAgentRespons // for reuse validation. It is not a physical worktree, so do not copy // the environment-level workspace path (which may be the host's seed // checkout) into the physical-worktree fields. - WorktreeID: worktreeID, - WorktreePath: worktreePath, - WorktreeBranch: worktreeBranch, - Position: 0, + WorktreeID: worktreeID, + WorktreePath: worktreePath, + WorktreeBranch: worktreeBranch, + WorktreeBranchOwner: worktreeBranchOwner, + WorktreeIntegrationRef: worktreeIntegrationRef, + Position: 0, }} } @@ -3045,13 +3052,15 @@ func buildTaskEnvironmentRepos(worktrees []RepoWorktreeResult) []*models.TaskEnv out := make([]*models.TaskEnvironmentRepo, 0, len(worktrees)) for i, w := range worktrees { out = append(out, &models.TaskEnvironmentRepo{ - RepositoryID: w.RepositoryID, - BranchSlug: w.BranchSlug, - WorktreeID: w.WorktreeID, - WorktreePath: w.WorktreePath, - WorktreeBranch: w.WorktreeBranch, - Position: i, - ErrorMessage: w.ErrorMessage, + RepositoryID: w.RepositoryID, + BranchSlug: w.BranchSlug, + WorktreeID: w.WorktreeID, + WorktreePath: w.WorktreePath, + WorktreeBranch: w.WorktreeBranch, + WorktreeBranchOwner: w.WorktreeBranchOwner, + WorktreeIntegrationRef: w.WorktreeIntegrationRef, + Position: i, + ErrorMessage: w.ErrorMessage, }) } return out @@ -3133,14 +3142,16 @@ func (e *Executor) persistOneTaskEnvironmentRepoTransition( return e.refreshTaskEnvironmentRepo(ctx, row, w, position, replacePhysical) } row = &models.TaskEnvironmentRepo{ - TaskEnvironmentID: envID, - RepositoryID: w.RepositoryID, - BranchSlug: w.BranchSlug, - WorktreeID: w.WorktreeID, - WorktreePath: w.WorktreePath, - WorktreeBranch: w.WorktreeBranch, - Position: position, - ErrorMessage: w.ErrorMessage, + TaskEnvironmentID: envID, + RepositoryID: w.RepositoryID, + BranchSlug: w.BranchSlug, + WorktreeID: w.WorktreeID, + WorktreePath: w.WorktreePath, + WorktreeBranch: w.WorktreeBranch, + WorktreeBranchOwner: w.WorktreeBranchOwner, + WorktreeIntegrationRef: w.WorktreeIntegrationRef, + Position: position, + ErrorMessage: w.ErrorMessage, } if createErr := e.repo.CreateTaskEnvironmentRepo(ctx, row); createErr != nil { e.logger.Warn("failed to persist task environment repo", @@ -3187,6 +3198,12 @@ func (e *Executor) refreshTaskEnvironmentRepo(ctx context.Context, row, w *model row.WorktreePath = w.WorktreePath row.WorktreeBranch = w.WorktreeBranch } + if w.WorktreeBranchOwner != "" || w.WorktreeID == "" { + row.WorktreeBranchOwner = w.WorktreeBranchOwner + } + if w.WorktreeIntegrationRef != "" || w.WorktreeID == "" || replacePhysical { + row.WorktreeIntegrationRef = w.WorktreeIntegrationRef + } row.Position = position row.ErrorMessage = w.ErrorMessage if replacePhysical { @@ -3195,6 +3212,8 @@ func (e *Executor) refreshTaskEnvironmentRepo(ctx context.Context, row, w *model // canonical inventory does not remain permanently excluded from reuse. row.Status = taskEnvironmentRepoStatusActive row.DeletedAt = nil + row.WorktreeRecoveryHeadSHA = "" + row.WorktreeBranchCompactedAt = nil } if err := e.repo.UpdateTaskEnvironmentRepo(ctx, row); err != nil { e.logger.Warn("failed to update task environment repo", @@ -3213,6 +3232,8 @@ func taskEnvironmentRepoNeedsRefresh(row, w *models.TaskEnvironmentRepo, positio (row.WorktreeID != w.WorktreeID || row.WorktreePath != w.WorktreePath || row.WorktreeBranch != w.WorktreeBranch)) || + (w.WorktreeBranchOwner != "" && row.WorktreeBranchOwner != w.WorktreeBranchOwner) || + ((w.WorktreeIntegrationRef != "" || replacePhysical) && row.WorktreeIntegrationRef != w.WorktreeIntegrationRef) || row.Position != position || row.ErrorMessage != w.ErrorMessage } diff --git a/apps/backend/internal/orchestrator/executor/executor_pr_base_resolver_test.go b/apps/backend/internal/orchestrator/executor/executor_pr_base_resolver_test.go index c0503cc986..02464dce05 100644 --- a/apps/backend/internal/orchestrator/executor/executor_pr_base_resolver_test.go +++ b/apps/backend/internal/orchestrator/executor/executor_pr_base_resolver_test.go @@ -47,6 +47,31 @@ func TestResolveTaskRepoInfo_PRBaseLookupFailureKeepsStoredBase(t *testing.T) { } } +func TestResolveTaskRepoInfo_FallsBackToTaskRepositoryBaseForIntegrationRef(t *testing.T) { + repo := newMockRepository() + repo.repositories["repo-1"] = &models.Repository{ + ID: "repo-1", + WorkspaceID: "workspace-1", + SourceType: sourceTypeLocal, + LocalPath: t.TempDir(), + DefaultBranch: "main", + } + exec := newTestExecutor(t, &mockAgentManager{}, repo) + + info, err := exec.resolveTaskRepoInfo(context.Background(), &models.TaskRepository{ + ID: "task-repo-1", + TaskID: "task-1", + RepositoryID: "repo-1", + BaseBranch: "main", + }) + if err != nil { + t.Fatalf("resolveTaskRepoInfo() error: %v", err) + } + if info.IntegrationRef != "main" { + t.Fatalf("IntegrationRef = %q, want task repository base", info.IntegrationRef) + } +} + // @covers AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.11 func TestResolveTaskRepoInfo_SkipsPRBaseLookupWithoutGitHubPR(t *testing.T) { tests := []struct { diff --git a/apps/backend/internal/orchestrator/executor/executor_resume.go b/apps/backend/internal/orchestrator/executor/executor_resume.go index 619fd98438..8096d72acf 100644 --- a/apps/backend/internal/orchestrator/executor/executor_resume.go +++ b/apps/backend/internal/orchestrator/executor/executor_resume.go @@ -68,6 +68,7 @@ type repoInfo struct { RepositoryID string RepositoryPath string BaseBranch string + IntegrationRef string CheckoutBranch string PRNumber int // GitHub PR number when CheckoutBranch is a PR head; sourced from task_repositories.metadata["pr_number"]. RemoteContribution *models.RemoteContribution @@ -170,6 +171,7 @@ func (e *Executor) resolveTaskRepoInfoForSession( TaskRepositoryID: tr.ID, RepositoryID: tr.RepositoryID, BaseBranch: tr.BaseBranch, + IntegrationRef: tr.BranchPolicyPullRequestTarget, CheckoutBranch: tr.CheckoutBranch, PRNumber: prNumberFromMetadata(tr.Metadata), Position: tr.Position, @@ -239,6 +241,9 @@ func (e *Executor) resolveTaskRepoInfoForSession( if info.BaseBranch == "" && repo.DefaultBranch != "" { info.BaseBranch = repo.DefaultBranch } + if info.IntegrationRef == "" { + info.IntegrationRef = info.BaseBranch + } if info.PullBeforeWorktree { refreshRequired, refreshErr := e.shouldRefreshRepositoryForSession(ctx, repo) if refreshErr != nil { diff --git a/apps/backend/internal/orchestrator/executor/executor_worktree_resume_test.go b/apps/backend/internal/orchestrator/executor/executor_worktree_resume_test.go index 1b033d12ea..f9ffce3b87 100644 --- a/apps/backend/internal/orchestrator/executor/executor_worktree_resume_test.go +++ b/apps/backend/internal/orchestrator/executor/executor_worktree_resume_test.go @@ -84,3 +84,65 @@ func TestLaunchPreparedSession_SingleRepoWorktree_PersistsSubdirAsWorkspacePath( env.WorkspacePath, subdir) } } + +// @covers AC-TASKS-MANAGED-BRANCH-COMPACTION-001.1 +func TestLaunchPreparedSession_SingleRepoWorktreePersistsBaseIntegrationRefForCompaction(t *testing.T) { + repo := newMockRepository() + const ( + taskID = "task-wt-integration-ref" + sessionID = "session-wt-integration-ref" + ) + repo.repositories["repo-only"] = &models.Repository{ + ID: "repo-only", Name: "only", LocalPath: "/repos/only", DefaultBranch: "main", + } + repo.taskRepositories["tr-only"] = &models.TaskRepository{ + ID: "tr-only", TaskID: taskID, RepositoryID: "repo-only", Position: 0, BaseBranch: "main", + } + repo.sessions[sessionID] = &models.TaskSession{ + ID: sessionID, TaskID: taskID, AgentProfileID: "profile-123", + State: models.TaskSessionStateCreated, StartedAt: time.Now(), UpdatedAt: time.Now(), + } + + var launchedIntegrationRef string + agentManager := &mockAgentManager{ + launchAgentFunc: func(_ context.Context, req *LaunchAgentRequest) (*LaunchAgentResponse, error) { + launchedIntegrationRef = req.IntegrationRef + return &LaunchAgentResponse{ + AgentExecutionID: "exec-integration-ref", + WorktreeID: "wt-integration-ref", + WorktreePath: "/tasks/integration-ref/only", + WorktreeBranch: "feature/integration-ref", + WorktreeBranchOwner: "kandev", + WorktreeIntegrationRef: req.IntegrationRef, + PrepareResult: &lifecycle.EnvPrepareResult{ + Success: true, WorktreeID: "wt-integration-ref", + }, + }, nil + }, + } + exec := newTestExecutor(t, agentManager, repo) + + task := &v1.Task{ID: taskID, WorkspaceID: "ws-1", Title: "Integration ref"} + if _, err := exec.LaunchPreparedSession(context.Background(), task, sessionID, LaunchOptions{ + AgentProfileID: "profile-123", + StartAgent: false, + }); err != nil { + t.Fatalf("LaunchPreparedSession: %v", err) + } + if launchedIntegrationRef != "main" { + t.Fatalf("launch IntegrationRef = %q, want task repository base", launchedIntegrationRef) + } + + var envID string + for id := range repo.taskEnvironments { + envID = id + } + rows := repo.taskEnvironmentRepos[envID] + if len(rows) != 1 { + t.Fatalf("persisted environment repositories = %d, want 1", len(rows)) + } + if rows[0].WorktreeBranchOwner != "kandev" || rows[0].WorktreeIntegrationRef != "main" { + t.Fatalf("persisted compaction metadata = owner %q, integration ref %q; want kandev/main", + rows[0].WorktreeBranchOwner, rows[0].WorktreeIntegrationRef) + } +} diff --git a/apps/backend/internal/orchestrator/executor/task_repository_identity_test.go b/apps/backend/internal/orchestrator/executor/task_repository_identity_test.go index 3e25a2cd5c..8323cf2844 100644 --- a/apps/backend/internal/orchestrator/executor/task_repository_identity_test.go +++ b/apps/backend/internal/orchestrator/executor/task_repository_identity_test.go @@ -6,15 +6,17 @@ import ( "github.com/stretchr/testify/require" ) -func TestBuildRepoSpecsCarriesExactTaskRepositoryID(t *testing.T) { +func TestBuildRepoSpecsCarriesTaskRepositoryIdentityAndIntegrationRef(t *testing.T) { specs := buildRepoSpecs([]*repoInfo{ - {RepositoryID: "repository-1", TaskRepositoryID: "task-repository-1", RepositoryPath: "/repo"}, - {RepositoryID: "repository-1", TaskRepositoryID: "task-repository-2", RepositoryPath: "/repo"}, + {RepositoryID: "repository-1", TaskRepositoryID: "task-repository-1", RepositoryPath: "/repo", IntegrationRef: "main"}, + {RepositoryID: "repository-1", TaskRepositoryID: "task-repository-2", RepositoryPath: "/repo", IntegrationRef: "release"}, }) require.Len(t, specs, 2) require.Equal(t, "task-repository-1", specs[0].TaskRepositoryID) require.Equal(t, "task-repository-2", specs[1].TaskRepositoryID) + require.Equal(t, "main", specs[0].IntegrationRef) + require.Equal(t, "release", specs[1].IntegrationRef) } func TestTopLevelLaunchRepoSpecCarriesExactTaskRepositoryID(t *testing.T) { @@ -22,8 +24,10 @@ func TestTopLevelLaunchRepoSpecCarriesExactTaskRepositoryID(t *testing.T) { RepositoryID: "repository-1", TaskRepositoryID: "task-repository-1", RepositoryPath: "/repo", + IntegrationRef: "develop", }) require.True(t, ok) require.Equal(t, "task-repository-1", spec.TaskRepositoryID) + require.Equal(t, "develop", spec.IntegrationRef) } diff --git a/apps/backend/internal/task/models/models.go b/apps/backend/internal/task/models/models.go index b6e9b19379..274ab0d6dd 100644 --- a/apps/backend/internal/task/models/models.go +++ b/apps/backend/internal/task/models/models.go @@ -2398,20 +2398,24 @@ func (te *TaskEnvironment) RepoFor(repositoryID string) *TaskEnvironmentRepo { // physical-worktree truth — identity, path, branch, status, and lifecycle // timestamps. type TaskEnvironmentRepo struct { - ID string `json:"id"` - TaskEnvironmentID string `json:"task_environment_id"` - RepositoryID string `json:"repository_id"` - BranchSlug string `json:"branch_slug,omitempty"` - WorktreeID string `json:"worktree_id,omitempty"` - WorktreePath string `json:"worktree_path,omitempty"` - WorktreeBranch string `json:"worktree_branch,omitempty"` - Position int `json:"position"` - ErrorMessage string `json:"error_message,omitempty"` - Status string `json:"status,omitempty"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` - MergedAt *time.Time `json:"merged_at,omitempty"` - DeletedAt *time.Time `json:"deleted_at,omitempty"` + ID string `json:"id"` + TaskEnvironmentID string `json:"task_environment_id"` + RepositoryID string `json:"repository_id"` + BranchSlug string `json:"branch_slug,omitempty"` + WorktreeID string `json:"worktree_id,omitempty"` + WorktreePath string `json:"worktree_path,omitempty"` + WorktreeBranch string `json:"worktree_branch,omitempty"` + WorktreeBranchOwner string `json:"-"` + WorktreeIntegrationRef string `json:"-"` + WorktreeRecoveryHeadSHA string `json:"-"` + WorktreeBranchCompactedAt *time.Time `json:"-"` + Position int `json:"position"` + ErrorMessage string `json:"error_message,omitempty"` + Status string `json:"status,omitempty"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` + MergedAt *time.Time `json:"merged_at,omitempty"` + DeletedAt *time.Time `json:"deleted_at,omitempty"` } // TaskEnvironmentRecoveryClaimRequest identifies the environment authority diff --git a/apps/backend/internal/task/repository/sqlite/base_migrations.go b/apps/backend/internal/task/repository/sqlite/base_migrations.go index aeba76d806..d69bc4a174 100644 --- a/apps/backend/internal/task/repository/sqlite/base_migrations.go +++ b/apps/backend/internal/task/repository/sqlite/base_migrations.go @@ -154,6 +154,10 @@ func (r *Repository) runMigrations() error { if err := r.migrateTaskEnvironmentReposAllowMultiBranch(); err != nil { return err } + _ = r.migrate.Apply("task_environment_repos.worktree_branch_owner", `ALTER TABLE task_environment_repos ADD COLUMN worktree_branch_owner TEXT NOT NULL DEFAULT 'unknown'`) + _ = r.migrate.Apply("task_environment_repos.worktree_integration_ref", `ALTER TABLE task_environment_repos ADD COLUMN worktree_integration_ref TEXT NOT NULL DEFAULT ''`) + _ = r.migrate.Apply("task_environment_repos.worktree_recovery_head_sha", `ALTER TABLE task_environment_repos ADD COLUMN worktree_recovery_head_sha TEXT NOT NULL DEFAULT ''`) + _ = r.migrate.Apply("task_environment_repos.worktree_branch_compacted_at", `ALTER TABLE task_environment_repos ADD COLUMN worktree_branch_compacted_at TIMESTAMP`) r.migrate.Apply("workflows.sort_order", `ALTER TABLE workflows ADD COLUMN sort_order INTEGER NOT NULL DEFAULT 0`) r.migrate.Apply("workflows.agent_profile_id", `ALTER TABLE workflows ADD COLUMN agent_profile_id TEXT DEFAULT ''`) r.migrate.Apply("workflows.hidden", `ALTER TABLE workflows ADD COLUMN hidden INTEGER NOT NULL DEFAULT 0`) diff --git a/apps/backend/internal/task/repository/sqlite/base_schema.go b/apps/backend/internal/task/repository/sqlite/base_schema.go index 0863e6d5b8..99a02d2ffa 100644 --- a/apps/backend/internal/task/repository/sqlite/base_schema.go +++ b/apps/backend/internal/task/repository/sqlite/base_schema.go @@ -53,6 +53,7 @@ func (r *Repository) initSchemaContext(ctx context.Context) error { r.healBuiltinWorkflowStepOnAgentError, r.normalizeTaskWorktreeOwnership, r.ensureTaskEnvironmentRecoveryClaimsSchema, + r.ensureArchivedBranchCandidatesIndex, r.healDuplicateTaskEnvironments, r.ensureTaskEnvironmentTaskUniqueIndex, r.healSessionTaskEnvironmentIDs, @@ -111,6 +112,23 @@ func (r *Repository) ensureTaskEnvironmentRecoveryClaimsSchema() error { return nil } +// ensureArchivedBranchCandidatesIndex runs after ownership normalization so +// every supported schema shape has the lifecycle columns the index requires. +func (r *Repository) ensureArchivedBranchCandidatesIndex() error { + _, err := r.db.Exec(` + CREATE INDEX IF NOT EXISTS idx_task_environment_repos_archived_branch_candidates + ON task_environment_repos( + worktree_branch_owner, + worktree_branch_compacted_at, + status, + deleted_at, + updated_at, + worktree_id, + task_environment_id + )`) + return err +} + func (r *Repository) initDynamicRoutingSchema() error { _, err := r.db.ExecContext(r.migrationContext(), fmt.Sprintf(` CREATE TABLE IF NOT EXISTS dynamic_route_states ( @@ -1156,6 +1174,10 @@ const sessionWorktreeSchemaDDL = ` worktree_id TEXT DEFAULT '', worktree_path TEXT DEFAULT '', worktree_branch TEXT DEFAULT '', + worktree_branch_owner TEXT NOT NULL DEFAULT 'unknown', + worktree_integration_ref TEXT NOT NULL DEFAULT '', + worktree_recovery_head_sha TEXT NOT NULL DEFAULT '', + worktree_branch_compacted_at TIMESTAMP, position INTEGER DEFAULT 0, error_message TEXT DEFAULT '', status TEXT NOT NULL DEFAULT 'active', diff --git a/apps/backend/internal/task/repository/sqlite/task_environment.go b/apps/backend/internal/task/repository/sqlite/task_environment.go index cd463884d4..7caf4adfc4 100644 --- a/apps/backend/internal/task/repository/sqlite/task_environment.go +++ b/apps/backend/internal/task/repository/sqlite/task_environment.go @@ -652,6 +652,12 @@ func (r *Repository) updateTaskEnvironmentRepoTransitionTx( row.WorktreePath = incoming.WorktreePath row.WorktreeBranch = incoming.WorktreeBranch } + if incoming.WorktreeBranchOwner != "" || incoming.WorktreeID == "" { + row.WorktreeBranchOwner = incoming.WorktreeBranchOwner + } + if incoming.WorktreeIntegrationRef != "" || incoming.WorktreeID == "" || replacePhysical { + row.WorktreeIntegrationRef = incoming.WorktreeIntegrationRef + } row.Position = position row.ErrorMessage = incoming.ErrorMessage // A later successful transition can recreate a slot that an earlier @@ -661,14 +667,20 @@ func (r *Repository) updateTaskEnvironmentRepoTransitionTx( if replacePhysical { row.Status = worktreeRepoStatusActive row.DeletedAt = nil + row.WorktreeRecoveryHeadSHA = "" + row.WorktreeBranchCompactedAt = nil } row.UpdatedAt = time.Now().UTC() _, err := tx.ExecContext(ctx, r.db.Rebind(` UPDATE task_environment_repos SET branch_slug = ?, worktree_id = ?, worktree_path = ?, worktree_branch = ?, + worktree_branch_owner = ?, worktree_integration_ref = ?, + worktree_recovery_head_sha = ?, worktree_branch_compacted_at = ?, position = ?, error_message = ?, status = ?, deleted_at = ?, updated_at = ? WHERE id = ? `), row.BranchSlug, row.WorktreeID, row.WorktreePath, row.WorktreeBranch, + row.WorktreeBranchOwner, row.WorktreeIntegrationRef, + row.WorktreeRecoveryHeadSHA, row.WorktreeBranchCompactedAt, row.Position, row.ErrorMessage, row.Status, row.DeletedAt, row.UpdatedAt, row.ID) return err } @@ -959,11 +971,15 @@ func (r *Repository) CreateTaskEnvironmentRepo(ctx context.Context, repo *models INSERT INTO task_environment_repos ( id, task_environment_id, repository_id, branch_slug, worktree_id, worktree_path, worktree_branch, + worktree_branch_owner, worktree_integration_ref, worktree_recovery_head_sha, + worktree_branch_compacted_at, position, error_message, status, created_at, updated_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `), repo.ID, repo.TaskEnvironmentID, repo.RepositoryID, repo.BranchSlug, repo.WorktreeID, repo.WorktreePath, repo.WorktreeBranch, + repo.WorktreeBranchOwner, repo.WorktreeIntegrationRef, repo.WorktreeRecoveryHeadSHA, + repo.WorktreeBranchCompactedAt, repo.Position, repo.ErrorMessage, repo.Status, repo.CreatedAt, repo.UpdatedAt, ); err != nil { return err @@ -990,11 +1006,15 @@ func (r *Repository) insertTaskEnvironmentRepoTx(ctx context.Context, tx *sqlx.T INSERT INTO task_environment_repos ( id, task_environment_id, repository_id, branch_slug, worktree_id, worktree_path, worktree_branch, + worktree_branch_owner, worktree_integration_ref, worktree_recovery_head_sha, + worktree_branch_compacted_at, position, error_message, status, created_at, updated_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `), repo.ID, repo.TaskEnvironmentID, repo.RepositoryID, repo.BranchSlug, repo.WorktreeID, repo.WorktreePath, repo.WorktreeBranch, + repo.WorktreeBranchOwner, repo.WorktreeIntegrationRef, repo.WorktreeRecoveryHeadSHA, + repo.WorktreeBranchCompactedAt, repo.Position, repo.ErrorMessage, repo.Status, repo.CreatedAt, repo.UpdatedAt, ) return err @@ -1006,6 +1026,10 @@ func (r *Repository) ListTaskEnvironmentRepos(ctx context.Context, envID string) SELECT id, task_environment_id, repository_id, COALESCE(branch_slug, ''), worktree_id, worktree_path, worktree_branch, + COALESCE(worktree_branch_owner, 'unknown'), + COALESCE(worktree_integration_ref, ''), + COALESCE(worktree_recovery_head_sha, ''), + worktree_branch_compacted_at, position, error_message, COALESCE(status, ''), created_at, updated_at, merged_at, deleted_at FROM task_environment_repos @@ -1020,11 +1044,13 @@ func (r *Repository) ListTaskEnvironmentRepos(ctx context.Context, envID string) var out []*models.TaskEnvironmentRepo for rows.Next() { repo := &models.TaskEnvironmentRepo{} - var mergedAt, deletedAt sql.NullTime + var compactedAt, mergedAt, deletedAt sql.NullTime if err := rows.Scan( &repo.ID, &repo.TaskEnvironmentID, &repo.RepositoryID, &repo.BranchSlug, &repo.WorktreeID, &repo.WorktreePath, &repo.WorktreeBranch, + &repo.WorktreeBranchOwner, &repo.WorktreeIntegrationRef, &repo.WorktreeRecoveryHeadSHA, + &compactedAt, &repo.Position, &repo.ErrorMessage, &repo.Status, &repo.CreatedAt, &repo.UpdatedAt, &mergedAt, &deletedAt, ); err != nil { @@ -1034,6 +1060,10 @@ func (r *Repository) ListTaskEnvironmentRepos(ctx context.Context, envID string) t := mergedAt.Time repo.MergedAt = &t } + if compactedAt.Valid { + t := compactedAt.Time + repo.WorktreeBranchCompactedAt = &t + } if deletedAt.Valid { t := deletedAt.Time repo.DeletedAt = &t @@ -1066,11 +1096,15 @@ func (r *Repository) UpdateTaskEnvironmentRepo(ctx context.Context, repo *models UPDATE task_environment_repos SET branch_slug = ?, worktree_id = ?, worktree_path = ?, worktree_branch = ?, + worktree_branch_owner = ?, worktree_integration_ref = ?, worktree_recovery_head_sha = ?, + worktree_branch_compacted_at = ?, position = ?, error_message = ?, status = ?, merged_at = ?, deleted_at = ?, updated_at = ? WHERE id = ? `), repo.BranchSlug, repo.WorktreeID, repo.WorktreePath, repo.WorktreeBranch, + repo.WorktreeBranchOwner, repo.WorktreeIntegrationRef, repo.WorktreeRecoveryHeadSHA, + repo.WorktreeBranchCompactedAt, repo.Position, repo.ErrorMessage, repo.Status, repo.MergedAt, repo.DeletedAt, repo.UpdatedAt, repo.ID, diff --git a/apps/backend/internal/task/repository/sqlite/task_environment_test.go b/apps/backend/internal/task/repository/sqlite/task_environment_test.go index 16dd3cb99c..6f3bb1c41a 100644 --- a/apps/backend/internal/task/repository/sqlite/task_environment_test.go +++ b/apps/backend/internal/task/repository/sqlite/task_environment_test.go @@ -3,6 +3,7 @@ package sqlite import ( "context" "errors" + "strings" "sync" "testing" "time" @@ -427,6 +428,7 @@ func TestPersistTaskEnvironmentTransitionReconcilesInventoryAtomically(t *testin env.TaskDirName = "stale-transition-root" if err := repo.PersistTaskEnvironmentTransition(ctx, env, []*models.TaskEnvironmentRepo{{ RepositoryID: "repo-keep", BranchSlug: "main", WorktreeID: "wt-new", WorktreePath: "/workspace/new/repo", + WorktreeBranchOwner: "kandev", WorktreeIntegrationRef: "develop", }}, true); err != nil { t.Fatalf("PersistTaskEnvironmentTransition: %v", err) } @@ -445,8 +447,8 @@ func TestPersistTaskEnvironmentTransitionReconcilesInventoryAtomically(t *testin t.Fatalf("repository inventory = %#v, want active row plus tombstone", persisted.Repos) } for _, row := range persisted.Repos { - if row.ID == "keep" && row.WorktreeID != "wt-new" { - t.Fatalf("kept row = %#v, want refreshed worktree", row) + if row.ID == "keep" && (row.WorktreeID != "wt-new" || row.WorktreeBranchOwner != "kandev" || row.WorktreeIntegrationRef != "develop") { + t.Fatalf("kept row = %#v, want refreshed worktree safety metadata", row) } if row.ID == "remove" && (row.Status != worktreeRepoStatusDeleted || row.DeletedAt == nil) { t.Fatalf("removed row = %#v, want deleted tombstone", row) @@ -471,12 +473,85 @@ func TestPersistTaskEnvironmentTransitionReconcilesInventoryAtomically(t *testin } } -// TestPersistTaskEnvironmentTransitionUntrackedBranchResumeUpdatesSoleRowInPlace -// covers a non-worktree (local/local_pc) resume, which never stamps a base -// branch and so always transitions with an incoming empty BranchSlug. It must -// update the repository's sole existing canonical row in place rather than -// insert a duplicate empty-branch row. The test uses a repo-backed task and -// performs two transitions to model successive resumes. +func TestPersistTaskEnvironmentTransitionClearsCompactionMetadataOnPhysicalReplacement(t *testing.T) { + repo := newRepoForEntityTests(t) + ctx := context.Background() + seedWorkspace(t, repo, "workspace-transition-compaction") + archivedAt := time.Now().UTC().Add(-time.Hour) + if err := repo.CreateTask(ctx, &models.Task{ + ID: "task-transition-compaction", + WorkspaceID: "workspace-transition-compaction", + Title: "transition compaction", + ArchivedAt: &archivedAt, + }); err != nil { + t.Fatal(err) + } + if _, err := repo.db.ExecContext(ctx, repo.db.Rebind(`UPDATE tasks SET archived_at = ? WHERE id = ?`), archivedAt, "task-transition-compaction"); err != nil { + t.Fatalf("archive task: %v", err) + } + env := &models.TaskEnvironment{ + ID: "env-transition-compaction", TaskID: "task-transition-compaction", ExecutorType: string(models.ExecutorTypeLocal), + Status: models.TaskEnvironmentStatusReady, WorkspacePath: "/workspace/old", + } + if err := repo.CreateTaskEnvironment(ctx, env); err != nil { + t.Fatalf("CreateTaskEnvironment: %v", err) + } + compactedAt := time.Now().UTC().Add(-30 * time.Minute) + if err := repo.CreateTaskEnvironmentRepo(ctx, &models.TaskEnvironmentRepo{ + ID: "compacted-repo", TaskEnvironmentID: env.ID, RepositoryID: "repo-compacted", BranchSlug: "main", + WorktreeID: "wt-old", WorktreeBranch: "feature/old", WorktreeBranchOwner: "kandev", + WorktreeIntegrationRef: "main", WorktreeRecoveryHeadSHA: strings.Repeat("a", 40), + WorktreeBranchCompactedAt: &compactedAt, Status: worktreeRepoStatusDeleted, DeletedAt: &compactedAt, + }); err != nil { + t.Fatalf("CreateTaskEnvironmentRepo: %v", err) + } + + if err := repo.PersistTaskEnvironmentTransition(ctx, env, []*models.TaskEnvironmentRepo{{ + RepositoryID: "repo-compacted", BranchSlug: "main", WorktreeID: "wt-new", + WorktreePath: "/workspace/new/repo", WorktreeBranch: "feature/new", + WorktreeBranchOwner: "kandev", WorktreeIntegrationRef: "main", + }}, true); err != nil { + t.Fatalf("PersistTaskEnvironmentTransition: %v", err) + } + + persisted, err := repo.GetTaskEnvironment(ctx, env.ID) + if err != nil { + t.Fatalf("GetTaskEnvironment after replacement: %v", err) + } + if len(persisted.Repos) != 1 { + t.Fatalf("repository inventory after replacement = %#v, want one row", persisted.Repos) + } + replaced := persisted.Repos[0] + if replaced.Status != worktreeRepoStatusActive || replaced.DeletedAt != nil || replaced.WorktreeID != "wt-new" { + t.Fatalf("replaced row = %#v, want active replacement worktree", replaced) + } + if replaced.WorktreeRecoveryHeadSHA != "" || replaced.WorktreeBranchCompactedAt != nil { + t.Fatalf("replaced row kept stale compaction metadata: head=%q compacted_at=%v", replaced.WorktreeRecoveryHeadSHA, replaced.WorktreeBranchCompactedAt) + } + + if err := repo.PersistTaskEnvironmentTransition(ctx, env, nil, true); err != nil { + t.Fatalf("PersistTaskEnvironmentTransition tombstone replacement: %v", err) + } + var candidateCount int + if err := repo.db.QueryRowContext(ctx, repo.db.Rebind(` + SELECT COUNT(1) + FROM task_environment_repos ter + INNER JOIN task_environments te ON ter.task_environment_id = te.id + INNER JOIN tasks t ON te.task_id = t.id + WHERE ter.worktree_id = ? + AND t.archived_at IS NOT NULL + AND ter.status = ? + AND ter.deleted_at IS NOT NULL + AND ter.worktree_branch_owner = ? + AND ter.worktree_branch_compacted_at IS NULL + `), "wt-new", worktreeRepoStatusDeleted, "kandev").Scan(&candidateCount); err != nil { + t.Fatalf("query archived maintenance candidate predicate: %v", err) + } + if candidateCount != 1 { + t.Fatalf("archived maintenance candidates for replacement = %d, want 1", candidateCount) + } +} + func TestPersistTaskEnvironmentTransitionUntrackedBranchResumeUpdatesSoleRowInPlace(t *testing.T) { repo := newRepoForEntityTests(t) ctx := context.Background() diff --git a/apps/backend/internal/task/repository/sqlite/worktree_ownership_migration.go b/apps/backend/internal/task/repository/sqlite/worktree_ownership_migration.go index 9c7dfcc574..d9c4bbdc35 100644 --- a/apps/backend/internal/task/repository/sqlite/worktree_ownership_migration.go +++ b/apps/backend/internal/task/repository/sqlite/worktree_ownership_migration.go @@ -75,6 +75,10 @@ func finalTaskEnvironmentReposDDL(tableName, refTable string) string { worktree_id TEXT DEFAULT '', worktree_path TEXT DEFAULT '', worktree_branch TEXT DEFAULT '', + worktree_branch_owner TEXT NOT NULL DEFAULT 'unknown', + worktree_integration_ref TEXT NOT NULL DEFAULT '', + worktree_recovery_head_sha TEXT NOT NULL DEFAULT '', + worktree_branch_compacted_at TIMESTAMP, position INTEGER DEFAULT 0, error_message TEXT DEFAULT '', status TEXT NOT NULL DEFAULT 'active', diff --git a/apps/backend/internal/task/repository/sqlite/worktree_ownership_migration_test.go b/apps/backend/internal/task/repository/sqlite/worktree_ownership_migration_test.go index 5cbaf29a1c..7a9556306f 100644 --- a/apps/backend/internal/task/repository/sqlite/worktree_ownership_migration_test.go +++ b/apps/backend/internal/task/repository/sqlite/worktree_ownership_migration_test.go @@ -5,6 +5,7 @@ package sqlite import ( "context" "path/filepath" + "strings" "testing" "time" @@ -227,11 +228,16 @@ func TestCutover_FreshSchemaHasFinalShape(t *testing.T) { } } repoCols := tableColumnSet(t, sqlxDB, tableTaskEnvRepos) - for _, required := range []string{"status", "merged_at", "deleted_at"} { + for _, required := range []string{ + "status", "merged_at", "deleted_at", "worktree_branch_compacted_at", + } { if !repoCols[required] { t.Fatalf("fresh task_environment_repos missing %s", required) } } + if !indexExists(t, sqlxDB, "idx_task_environment_repos_archived_branch_candidates") { + t.Fatal("fresh task_environment_repos missing archived branch candidate index") + } } func TestCutover_HybridNormalizedEnvironmentWithLegacySessionWorktrees(t *testing.T) { @@ -282,8 +288,14 @@ func seedHybridCutoverState(t *testing.T, db *sqlx.DB, suffix string) legacySeed } if _, err := db.Exec(db.Rebind(` UPDATE task_environment_repos - SET status = 'deleted', merged_at = ?, deleted_at = ? - WHERE id = 'env-repo-hybrid'`), now, now); err != nil { + SET status = 'deleted', + worktree_branch_owner = 'kandev', + worktree_integration_ref = 'refs/heads/main', + worktree_recovery_head_sha = ?, + worktree_branch_compacted_at = ?, + merged_at = ?, + deleted_at = ? + WHERE id = 'env-repo-hybrid'`), strings.Repeat("a", 40), now, now, now); err != nil { t.Fatalf("seed normalized repository lifecycle: %v", err) } if _, err := db.Exec(legacySessionWorktreeDDL); err != nil { @@ -320,6 +332,12 @@ func assertHybridCutoverResult(t *testing.T, repo *Repository, seed legacySeed) !got[seed.repoID].DeletedAt.Equal(got[seed.repoID].CreatedAt) { t.Fatalf("normalized repository lifecycle = %+v", got[seed.repoID]) } + if got[seed.repoID].WorktreeBranchOwner != "kandev" || + got[seed.repoID].WorktreeIntegrationRef != "refs/heads/main" || + got[seed.repoID].WorktreeRecoveryHeadSHA != strings.Repeat("a", 40) || + got[seed.repoID].WorktreeBranchCompactedAt == nil { + t.Fatalf("normalized repository branch metadata = %+v", got[seed.repoID]) + } assertHybridWorktree(t, got[seed.repoID+"-session-only"], "wt-session-only", "/tasks/hybrid/session-only", "feature/session-only") var snapshotEnvironmentID string @@ -358,6 +376,17 @@ func tableColumnSet(t *testing.T, db *sqlx.DB, table string) map[string]bool { return cols } +func indexExists(t *testing.T, db *sqlx.DB, index string) bool { + t.Helper() + var exists bool + err := db.QueryRow( + `SELECT EXISTS (SELECT 1 FROM sqlite_master WHERE type='index' AND name=?)`, index).Scan(&exists) + if err != nil { + t.Fatalf("probe index: %v", err) + } + return exists +} + // --------------------------------------------------------------------------- // Legacy normalization // --------------------------------------------------------------------------- diff --git a/apps/backend/internal/task/repository/sqlite/worktree_ownership_normalize.go b/apps/backend/internal/task/repository/sqlite/worktree_ownership_normalize.go index 6dbc870fc7..21218446e8 100644 --- a/apps/backend/internal/task/repository/sqlite/worktree_ownership_normalize.go +++ b/apps/backend/internal/task/repository/sqlite/worktree_ownership_normalize.go @@ -61,12 +61,14 @@ type legacySession struct { } type legacyEnvRepo struct { - id, envID, repositoryID, branchSlug string - worktreeID, worktreePath, worktreeBranch string - position int - errorMessage, status string - mergedAt, deletedAt *time.Time - createdAt, updatedAt time.Time + id, envID, repositoryID, branchSlug string + worktreeID, worktreePath, worktreeBranch string + worktreeBranchOwner, worktreeIntegrationRef, worktreeRecoveryHeadSHA string + worktreeBranchCompactedAt *time.Time + position int + errorMessage, status string + mergedAt, deletedAt *time.Time + createdAt, updatedAt time.Time } type legacySessionWorktree struct { @@ -191,9 +193,13 @@ func (c *worktreeCutover) loadLegacyEnvRepos(tx *sqlx.Tx, columns map[string]boo rows, err := tx.QueryContext(c.ctx, fmt.Sprintf(` SELECT id, task_environment_id, repository_id, COALESCE(branch_slug, ''), COALESCE(worktree_id, ''), COALESCE(worktree_path, ''), - COALESCE(worktree_branch, ''), position, COALESCE(error_message, ''), - %s, created_at, updated_at, %s, %s + COALESCE(worktree_branch, ''), %s, %s, %s, %s, position, COALESCE(error_message, ''), + %s, created_at, updated_at, %s, %s FROM task_environment_repos`, + legacyRepoColumnExpr(columns, "worktree_branch_owner", "CAST('unknown' AS TEXT)"), + legacyRepoColumnExpr(columns, "worktree_integration_ref", "CAST('' AS TEXT)"), + legacyRepoColumnExpr(columns, "worktree_recovery_head_sha", "CAST('' AS TEXT)"), + legacyRepoColumnExpr(columns, "worktree_branch_compacted_at", "NULL"), legacyRepoColumnExpr(columns, "status", "CAST('active' AS TEXT)"), legacyRepoColumnExpr(columns, "merged_at", "NULL"), legacyRepoColumnExpr(columns, "deleted_at", "NULL"))) @@ -203,10 +209,11 @@ func (c *worktreeCutover) loadLegacyEnvRepos(tx *sqlx.Tx, columns map[string]boo defer func() { _ = rows.Close() }() for rows.Next() { var row legacyEnvRepo - var mergedAt, deletedAt sql.NullTime + var mergedAt, deletedAt, compactedAt sql.NullTime if err := rows.Scan(&row.id, &row.envID, &row.repositoryID, &row.branchSlug, - &row.worktreeID, &row.worktreePath, &row.worktreeBranch, &row.position, - &row.errorMessage, &row.status, &row.createdAt, &row.updatedAt, &mergedAt, &deletedAt); err != nil { + &row.worktreeID, &row.worktreePath, &row.worktreeBranch, + &row.worktreeBranchOwner, &row.worktreeIntegrationRef, &row.worktreeRecoveryHeadSHA, + &compactedAt, &row.position, &row.errorMessage, &row.status, &row.createdAt, &row.updatedAt, &mergedAt, &deletedAt); err != nil { return fmt.Errorf("cutover: scan legacy environment repo: %w", err) } if mergedAt.Valid { @@ -217,6 +224,10 @@ func (c *worktreeCutover) loadLegacyEnvRepos(tx *sqlx.Tx, columns map[string]boo t := deletedAt.Time row.deletedAt = &t } + if compactedAt.Valid { + t := compactedAt.Time + row.worktreeBranchCompactedAt = &t + } c.envRepos = append(c.envRepos, row) } return rows.Err() @@ -274,17 +285,21 @@ type taskWorktreeTargets struct { } type envRepoTarget struct { - key string - repositoryID, branchSlug string - canonical bool - worktreeID string - worktreePath string - worktreeBranch string - position int - errorMessage string - status string - mergedAt, deletedAt *time.Time - createdAt, updatedAt time.Time + key string + repositoryID, branchSlug string + canonical bool + worktreeID string + worktreePath string + worktreeBranch string + worktreeBranchOwner string + worktreeIntegrationRef string + worktreeRecoveryHeadSHA string + worktreeBranchCompactedAt *time.Time + position int + errorMessage string + status string + mergedAt, deletedAt *time.Time + createdAt, updatedAt time.Time } // normalize builds the final ownership graph in memory: one environment per diff --git a/apps/backend/internal/task/repository/sqlite/worktree_ownership_targets.go b/apps/backend/internal/task/repository/sqlite/worktree_ownership_targets.go index 227b7f9567..2afed70c9f 100644 --- a/apps/backend/internal/task/repository/sqlite/worktree_ownership_targets.go +++ b/apps/backend/internal/task/repository/sqlite/worktree_ownership_targets.go @@ -85,6 +85,10 @@ func (t *taskWorktreeTargets) mergeLegacyEnvRepo(row legacyEnvRepo) error { } target.mergedAt = row.mergedAt target.deletedAt = row.deletedAt + target.worktreeBranchOwner = row.worktreeBranchOwner + target.worktreeIntegrationRef = row.worktreeIntegrationRef + target.worktreeRecoveryHeadSHA = row.worktreeRecoveryHeadSHA + target.worktreeBranchCompactedAt = row.worktreeBranchCompactedAt return target.mergeCreation(row.createdAt, row.updatedAt) } @@ -296,11 +300,13 @@ func (c *worktreeCutover) insertRepoRow(tx *sqlx.Tx, taskID, envID string, targe if _, err := tx.ExecContext(c.ctx, tx.Rebind(` INSERT INTO task_environment_repos_shadow ( id, task_environment_id, repository_id, branch_slug, - worktree_id, worktree_path, worktree_branch, position, + worktree_id, worktree_path, worktree_branch, worktree_branch_owner, + worktree_integration_ref, worktree_recovery_head_sha, worktree_branch_compacted_at, position, error_message, status, created_at, updated_at, merged_at, deleted_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`), + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`), uuid.New().String(), envID, target.repositoryID, target.branchSlug, - target.worktreeID, target.worktreePath, target.worktreeBranch, target.position, + target.worktreeID, target.worktreePath, target.worktreeBranch, target.worktreeBranchOwner, + target.worktreeIntegrationRef, target.worktreeRecoveryHeadSHA, target.worktreeBranchCompactedAt, target.position, target.errorMessage, target.status, target.createdAt, target.updatedAt, mergedAt, deletedAt); err != nil { return fmt.Errorf("cutover: insert shadow repo row for task %s: %w", taskID, err) @@ -447,7 +453,9 @@ func (r *Repository) checkShadowFinalSchema(tx *sqlx.Tx) error { return err } for _, required := range []string{"id", "task_environment_id", columnRepositoryID, "branch_slug", - columnWorktreeID, columnWorktreePath, columnWorktreeBranch, "position", "error_message", + columnWorktreeID, columnWorktreePath, columnWorktreeBranch, + "worktree_branch_owner", "worktree_integration_ref", "worktree_recovery_head_sha", + "worktree_branch_compacted_at", "position", "error_message", columnStatus, columnCreatedAt, columnUpdatedAt, "merged_at", "deleted_at"} { if !repoColumns[required] { return fmt.Errorf("cutover: task_environment_repos_shadow missing final column %s", required) @@ -532,6 +540,7 @@ func (r *Repository) cutoverSwap(c *worktreeCutover, tx *sqlx.Tx) error { `CREATE INDEX IF NOT EXISTS idx_task_environments_status ON task_environments(status)`, `CREATE INDEX IF NOT EXISTS idx_task_environment_repos_env_id ON task_environment_repos(task_environment_id)`, `CREATE INDEX IF NOT EXISTS idx_task_environment_repos_repository_id ON task_environment_repos(repository_id)`, + `CREATE INDEX IF NOT EXISTS idx_task_environment_repos_archived_branch_candidates ON task_environment_repos(worktree_branch_owner, worktree_branch_compacted_at, status, deleted_at, updated_at, worktree_id, task_environment_id)`, } { if _, err := tx.ExecContext(r.migrationContext(), stmt); err != nil { return fmt.Errorf("cutover: recreate index: %w", err) diff --git a/apps/backend/internal/task/repository/task_environment_repo_test.go b/apps/backend/internal/task/repository/task_environment_repo_test.go index c578ccebd4..e07c512f71 100644 --- a/apps/backend/internal/task/repository/task_environment_repo_test.go +++ b/apps/backend/internal/task/repository/task_environment_repo_test.go @@ -3,7 +3,9 @@ package repository import ( "context" "path/filepath" + "strings" "testing" + "time" "github.com/jmoiron/sqlx" @@ -66,14 +68,19 @@ func TestTaskEnvironmentRepo_CRUD(t *testing.T) { newTaskWithRepo(t, repo, "task-env-2") env := newEnv(t, repo, "task-env-2", "env-2") + compactedAt := time.Now().UTC().Truncate(time.Second) er := &models.TaskEnvironmentRepo{ - TaskEnvironmentID: env.ID, - RepositoryID: "repo-frontend", - WorktreeID: "wt-1", - WorktreePath: "/tmp/tasks/x/frontend", - WorktreeBranch: "feature/x", - Position: 0, + TaskEnvironmentID: env.ID, + RepositoryID: "repo-frontend", + WorktreeID: "wt-1", + WorktreePath: "/tmp/tasks/x/frontend", + WorktreeBranch: "feature/x", + WorktreeBranchOwner: "kandev", + WorktreeIntegrationRef: "main", + WorktreeRecoveryHeadSHA: strings.Repeat("a", 40), + WorktreeBranchCompactedAt: &compactedAt, + Position: 0, } if err := repo.CreateTaskEnvironmentRepo(ctx, er); err != nil { t.Fatalf("create env repo: %v", err) @@ -86,17 +93,22 @@ func TestTaskEnvironmentRepo_CRUD(t *testing.T) { if err != nil { t.Fatalf("list: %v", err) } - if len(list) != 1 || list[0].RepositoryID != "repo-frontend" { + if len(list) != 1 || list[0].RepositoryID != "repo-frontend" || + list[0].WorktreeBranchOwner != "kandev" || list[0].WorktreeIntegrationRef != "main" || + list[0].WorktreeRecoveryHeadSHA != strings.Repeat("a", 40) || + list[0].WorktreeBranchCompactedAt == nil || + !list[0].WorktreeBranchCompactedAt.Equal(compactedAt) { t.Fatalf("unexpected list: %+v", list) } er.WorktreeBranch = "feature/x-renamed" + er.WorktreeIntegrationRef = "develop" er.ErrorMessage = "fetch failed" if err := repo.UpdateTaskEnvironmentRepo(ctx, er); err != nil { t.Fatalf("update: %v", err) } list, _ = repo.ListTaskEnvironmentRepos(ctx, env.ID) - if list[0].WorktreeBranch != "feature/x-renamed" || list[0].ErrorMessage != "fetch failed" { + if list[0].WorktreeBranch != "feature/x-renamed" || list[0].WorktreeIntegrationRef != "develop" || list[0].ErrorMessage != "fetch failed" { t.Errorf("update did not persist: %+v", list[0]) } diff --git a/apps/backend/internal/task/service/resource_cleanup_archive_identity_test.go b/apps/backend/internal/task/service/resource_cleanup_archive_identity_test.go index fe916c505f..24d08a7d3b 100644 --- a/apps/backend/internal/task/service/resource_cleanup_archive_identity_test.go +++ b/apps/backend/internal/task/service/resource_cleanup_archive_identity_test.go @@ -2,6 +2,7 @@ package service import ( "context" + "encoding/json" "errors" "path/filepath" "strings" @@ -234,6 +235,119 @@ func TestArchiveUnarchiveResumeReactivatesLocalOnlyBranch(t *testing.T) { } } +func TestArchiveCleanupSnapshotPreservesBranchMetadataAcrossRestart(t *testing.T) { + ctx := context.Background() + svc, _, repo := createTestService(t) + svc.StopTaskResourceCleanupWorker() + const ( + taskID = "task-archive-metadata-restart" + sessionID = "session-archive-metadata-restart" + repositoryID = "repo-archive-metadata-restart" + environmentID = "env-archive-metadata-restart" + ) + 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, DefaultBranch: "main", + }); err != nil { + t.Fatalf("CreateRepository: %v", err) + } + + manager := newCleanupTestWorktreeManager(t, repo) + manager.SetRepositoryProvider(worktree.NewRepositoryAdapter(repo)) + wt, err := manager.Create(ctx, worktree.CreateRequest{ + TaskID: taskID, SessionID: sessionID, TaskTitle: "Archive metadata restart", + RepositoryID: repositoryID, RepositoryPath: sourcePath, + BaseBranch: "main", IntegrationRef: "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-" + taskID, RepositoryID: repositoryID, + BranchSlug: wt.BranchSlug, WorktreeID: wt.ID, WorktreePath: wt.Path, + WorktreeBranch: wt.Branch, WorktreeBranchOwner: wt.BranchOwner, + WorktreeIntegrationRef: wt.IntegrationRef, 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) + } + + runGitTestCmd(t, wt.Path, "commit", "--allow-empty", "-m", "archive before integration") + wantHead := strings.TrimSpace(string(runGitTestCmd(t, wt.Path, "rev-parse", "HEAD"))) + svc.SetWorktreeCleanup(manager) + if err := svc.ArchiveTask(ctx, taskID); err != nil { + t.Fatalf("ArchiveTask: %v", err) + } + + job := latestCleanupJob(t, repo, taskID, models.TaskResourceCleanupTriggerArchive) + var snapshot struct { + WorktreeBranchMetadata map[string]struct { + BranchOwner string `json:"branch_owner,omitempty"` + IntegrationRef string `json:"integration_ref,omitempty"` + } `json:"worktree_branch_metadata,omitempty"` + } + if err := json.Unmarshal([]byte(job.ResourceSnapshot), &snapshot); err != nil { + t.Fatalf("decode archive snapshot: %v", err) + } + metadata, ok := snapshot.WorktreeBranchMetadata[wt.ID] + if !ok { + t.Fatalf("archive snapshot has no branch metadata for %s: %#v", wt.ID, snapshot.WorktreeBranchMetadata) + } + if metadata.BranchOwner != worktree.BranchOwnerManaged || metadata.IntegrationRef != "main" { + t.Fatalf("archive snapshot metadata = %+v, want managed/main", metadata) + } + + // Process the durable job with a fresh manager. This models a restart and + // proves that the JSON boundary, not the old manager cache, supplies the + // cleanup policy fields. + restartedManager := newCleanupTestWorktreeManager(t, repo) + restartedManager.SetRepositoryProvider(worktree.NewRepositoryAdapter(repo)) + svc.SetWorktreeCleanup(restartedManager) + svc.SetEnvironmentDestroyer(&archiveManagerEnvironmentDestroyer{mgr: restartedManager}) + if err := svc.processTaskResourceCleanupJob(ctx, job.ID); err != nil { + t.Fatalf("process archive cleanup: %v", err) + } + if got := strings.TrimSpace(string(runGitTestCmd(t, sourcePath, "rev-parse", wt.Branch))); got != wantHead { + t.Fatalf("archive retained branch head = %q, want %q", got, wantHead) + } + + // The archive happened before integration. Later maintenance must still + // find the durable row and compact the now-integrated branch. + runGitTestCmd(t, sourcePath, "update-ref", "refs/heads/main", wantHead) + if _, err := restartedManager.MaintainArchivedBranches(ctx, 1); err != nil { + t.Fatalf("maintain archived branches: %v", err) + } + if got := strings.TrimSpace(string(runGitTestCmd(t, sourcePath, "branch", "--list", wt.Branch))); got != "" { + t.Fatalf("integrated archived branch remains after maintenance: %q", got) + } + persisted, err := restartedManager.GetByID(ctx, wt.ID) + if err != nil { + t.Fatalf("load archived worktree: %v", err) + } + if persisted.BranchOwner != worktree.BranchOwnerManaged || persisted.IntegrationRef != "main" { + t.Fatalf("persisted branch metadata = owner %q integration %q, want managed/main", + persisted.BranchOwner, persisted.IntegrationRef) + } + if persisted.RecoveryHeadSHA != wantHead || persisted.BranchCompactedAt == nil { + t.Fatalf("persisted recovery state = head %q compacted %v, want %q and marker", + persisted.RecoveryHeadSHA, persisted.BranchCompactedAt, wantHead) + } +} + func TestCleanupTaskResources_CascadeArchivePreservesBranches(t *testing.T) { svc, _, repo := createTestService(t) const taskID = "task-cascade-archive-policy" diff --git a/apps/backend/internal/task/service/resource_cleanup_jobs.go b/apps/backend/internal/task/service/resource_cleanup_jobs.go index 40b773d132..8239e3fd43 100644 --- a/apps/backend/internal/task/service/resource_cleanup_jobs.go +++ b/apps/backend/internal/task/service/resource_cleanup_jobs.go @@ -57,18 +57,29 @@ type persistedTaskAttachment struct { StorageKey string `json:"storage_key"` } +// persistedWorktreeBranchMetadata carries internal branch-cleanup state +// through the durable cleanup snapshot. Worktree hides these fields from +// public JSON, but an archive job must restore them after a process restart. +type persistedWorktreeBranchMetadata struct { + BranchCompactedAt *time.Time `json:"branch_compacted_at,omitempty"` + BranchOwner string `json:"branch_owner,omitempty"` + IntegrationRef string `json:"integration_ref,omitempty"` + RecoveryHeadSHA string `json:"recovery_head_sha,omitempty"` +} + type taskResourceCleanupSnapshot struct { - Sessions []*models.TaskSession `json:"sessions,omitempty"` - Worktrees []*worktree.Worktree `json:"worktrees,omitempty"` - WorktreeHeadOIDs map[string]string `json:"worktree_head_oids,omitempty"` - WorktreeTaskDirNames map[string]string `json:"worktree_task_dir_names,omitempty"` - DiscardWorktreeChanges bool `json:"discard_worktree_changes,omitempty"` - StopTargets []persistedTaskStopTarget `json:"stop_targets,omitempty"` - TaskEnvironment *models.TaskEnvironment `json:"task_environment,omitempty"` - Attachments []persistedTaskAttachment `json:"attachments,omitempty"` - WorkspaceID string `json:"workspace_id,omitempty"` - DeleteEnvironmentRow bool `json:"delete_environment_row,omitempty"` - LegacyWorktreeCleanup bool `json:"legacy_worktree_cleanup,omitempty"` + Sessions []*models.TaskSession `json:"sessions,omitempty"` + Worktrees []*worktree.Worktree `json:"worktrees,omitempty"` + WorktreeHeadOIDs map[string]string `json:"worktree_head_oids,omitempty"` + WorktreeTaskDirNames map[string]string `json:"worktree_task_dir_names,omitempty"` + WorktreeBranchMetadata map[string]persistedWorktreeBranchMetadata `json:"worktree_branch_metadata,omitempty"` + DiscardWorktreeChanges bool `json:"discard_worktree_changes,omitempty"` + StopTargets []persistedTaskStopTarget `json:"stop_targets,omitempty"` + TaskEnvironment *models.TaskEnvironment `json:"task_environment,omitempty"` + Attachments []persistedTaskAttachment `json:"attachments,omitempty"` + WorkspaceID string `json:"workspace_id,omitempty"` + DeleteEnvironmentRow bool `json:"delete_environment_row,omitempty"` + LegacyWorktreeCleanup bool `json:"legacy_worktree_cleanup,omitempty"` // SSHTaskDirs records the remote task directories this task launched into. // Additive and absent-tolerant: a job row written by an older backend // decodes with an empty list and reclaims nothing. @@ -130,11 +141,13 @@ func (s *Service) persistTaskResourceCleanup( }) } worktreeTaskDirNames := captureWorktreeTaskDirNames(worktrees) + worktreeBranchMetadata := captureWorktreeBranchMetadata(worktrees) snapshot := taskResourceCleanupSnapshot{ Sessions: sessions, Worktrees: worktrees, WorktreeHeadOIDs: worktreeHeadOIDs, WorktreeTaskDirNames: worktreeTaskDirNames, + WorktreeBranchMetadata: worktreeBranchMetadata, DiscardWorktreeChanges: envCleanup.discardWorktreeChanges, StopTargets: persistStopTargets(stopTargets), Attachments: persistedAttachments, @@ -202,6 +215,32 @@ func captureWorktreeTaskDirNames(worktrees []*worktree.Worktree) map[string]stri return names } +func captureWorktreeBranchMetadata(worktrees []*worktree.Worktree) map[string]persistedWorktreeBranchMetadata { + if len(worktrees) == 0 { + return nil + } + metadata := make(map[string]persistedWorktreeBranchMetadata, len(worktrees)) + for _, wt := range worktrees { + if wt == nil || wt.ID == "" { + continue + } + if wt.BranchOwner == "" && wt.IntegrationRef == "" && + wt.RecoveryHeadSHA == "" && wt.BranchCompactedAt == nil { + continue + } + metadata[wt.ID] = persistedWorktreeBranchMetadata{ + BranchCompactedAt: wt.BranchCompactedAt, + BranchOwner: wt.BranchOwner, + IntegrationRef: wt.IntegrationRef, + RecoveryHeadSHA: wt.RecoveryHeadSHA, + } + } + if len(metadata) == 0 { + return nil + } + return metadata +} + func persistStopTargets(targets []taskStopTarget) []persistedTaskStopTarget { result := make([]persistedTaskStopTarget, 0, len(targets)) for _, target := range targets { @@ -477,6 +516,12 @@ func (s *Service) processTaskResourceCleanupJob(ctx context.Context, id string) if wt == nil { continue } + if metadata, found := snapshot.WorktreeBranchMetadata[wt.ID]; found { + wt.BranchCompactedAt = metadata.BranchCompactedAt + wt.BranchOwner = metadata.BranchOwner + wt.IntegrationRef = metadata.IntegrationRef + wt.RecoveryHeadSHA = metadata.RecoveryHeadSHA + } if snapshot.WorktreeHeadOIDs != nil { cleanupHeadOID, found := snapshot.WorktreeHeadOIDs[wt.ID] wt.CleanupHeadOID = cleanupHeadOID @@ -992,6 +1037,7 @@ func (s *Service) PrepareTaskResourceCleanupWithOptions( return cancelPrepared(err) } worktreeTaskDirNames := captureWorktreeTaskDirNames(worktrees) + worktreeBranchMetadata := captureWorktreeBranchMetadata(worktrees) taskEnv, err := s.gatherTaskEnvironmentForCleanup(ctx, taskID) if err != nil { return cancelPrepared(fmt.Errorf("lookup task environment for cleanup: %w", err)) @@ -1001,8 +1047,11 @@ func (s *Service) PrepareTaskResourceCleanupWithOptions( return cancelPrepared(fmt.Errorf("list remote task directories for cleanup snapshot: %w", err)) } snapshot := taskResourceCleanupSnapshot{ - Sessions: sessions, Worktrees: worktrees, WorktreeHeadOIDs: worktreeHeadOIDs, + Sessions: sessions, + Worktrees: worktrees, + WorktreeHeadOIDs: worktreeHeadOIDs, WorktreeTaskDirNames: worktreeTaskDirNames, + WorktreeBranchMetadata: worktreeBranchMetadata, StopTargets: persistStopTargets(stopTargets), TaskEnvironment: taskEnv, DeleteEnvironmentRow: deleteEnvironmentRow, diff --git a/apps/backend/internal/task/service/service_branch_recovery.go b/apps/backend/internal/task/service/service_branch_recovery.go index 208bb861f2..1fc759cd58 100644 --- a/apps/backend/internal/task/service/service_branch_recovery.go +++ b/apps/backend/internal/task/service/service_branch_recovery.go @@ -26,10 +26,14 @@ type BranchStatusProber interface { BranchRecoveryStatus(ctx context.Context, repoPath, branch string) string } +type ExactBranchStatusProber interface { + RecoverBranchStatus(ctx context.Context, wt *worktree.Worktree) string +} + // RecoverTaskBranches inspects the task's historical worktree records -// (archive keeps the rows and deletes only the local branch + directory) -// and, per repository, probes whether the branch still exists locally or -// on origin. When it does and the task_repositories row has no +// (archive keeps the rows and may safely compact an integrated local branch) +// and, per repository, restores an exact compacted head or probes whether the +// branch still exists locally or on origin. When it does and the task_repositories row has no // checkout_branch yet, the branch is written back as checkout_branch so a // brand-new session checks the old work back out instead of minting a // fresh branch. Best-effort: failures are logged and reported as @@ -55,6 +59,9 @@ func (s *Service) RecoverTaskBranches(ctx context.Context, taskID string) []Bran restored := false for _, wt := range latest { status := prober.BranchRecoveryStatus(ctx, wt.RepositoryPath, wt.Branch) + if exact, ok := s.worktreeCleanup.(ExactBranchStatusProber); ok { + status = exact.RecoverBranchStatus(ctx, wt) + } if status != worktree.BranchStatusMissing { restored = s.restoreCheckoutBranch(ctx, reposByID[wt.RepositoryID], wt.Branch) || restored } diff --git a/apps/backend/internal/task/service/service_branch_recovery_test.go b/apps/backend/internal/task/service/service_branch_recovery_test.go index 22c24a5d25..8596f71792 100644 --- a/apps/backend/internal/task/service/service_branch_recovery_test.go +++ b/apps/backend/internal/task/service/service_branch_recovery_test.go @@ -13,8 +13,9 @@ import ( // stubWorktreeRecovery implements WorktreeProvider + BranchStatusProber. type stubWorktreeRecovery struct { - worktrees []*worktree.Worktree - statuses map[string]string // branch → status + worktrees []*worktree.Worktree + statuses map[string]string // branch → status + exactStatuses map[string]string } func (s *stubWorktreeRecovery) OnTaskDeleted(context.Context, string) error { return nil } @@ -24,6 +25,12 @@ func (s *stubWorktreeRecovery) GetAllByTaskID(context.Context, string) ([]*workt func (s *stubWorktreeRecovery) BranchRecoveryStatus(_ context.Context, _, branch string) string { return s.statuses[branch] } +func (s *stubWorktreeRecovery) RecoverBranchStatus(_ context.Context, wt *worktree.Worktree) string { + if status := s.exactStatuses[wt.Branch]; status != "" { + return status + } + return s.statuses[wt.Branch] +} // stubTaskRepoRepo overrides just the two methods RecoverTaskBranches uses. type stubTaskRepoRepo struct { @@ -115,6 +122,27 @@ func TestRecoverTaskBranches_MissingBranchReportsWithoutUpdate(t *testing.T) { } } +func TestRecoverTaskBranches_UsesExactCompactedHeadRecovery(t *testing.T) { + wt := &stubWorktreeRecovery{ + worktrees: []*worktree.Worktree{{ + RepositoryID: "repo-1", RepositoryPath: "/repos/one", Branch: "feature/compacted", + BranchOwner: worktree.BranchOwnerManaged, RecoveryHeadSHA: "accepted-head", + }}, + statuses: map[string]string{"feature/compacted": worktree.BranchStatusMissing}, + exactStatuses: map[string]string{"feature/compacted": worktree.BranchStatusLocal}, + } + repos := &stubTaskRepoRepo{repos: []*models.TaskRepository{{TaskID: "task-1", RepositoryID: "repo-1"}}} + svc, _ := recoveryTestService(t, wt, repos) + + out := svc.RecoverTaskBranches(context.Background(), "task-1") + if len(out) != 1 || out[0].Status != worktree.BranchStatusLocal { + t.Fatalf("recovery = %+v, want exact local recovery", out) + } + if len(repos.updated) != 1 || repos.updated[0].CheckoutBranch != "feature/compacted" { + t.Fatalf("checkout branch was not restored after exact recovery: %+v", repos.updated) + } +} + func TestRecoverTaskBranches_NeverOverwritesExistingCheckoutBranch(t *testing.T) { wt := &stubWorktreeRecovery{ worktrees: []*worktree.Worktree{{ diff --git a/apps/backend/internal/task/service/service_task_environments.go b/apps/backend/internal/task/service/service_task_environments.go index e87fd27b1d..297da2a56e 100644 --- a/apps/backend/internal/task/service/service_task_environments.go +++ b/apps/backend/internal/task/service/service_task_environments.go @@ -381,11 +381,21 @@ func (s *Service) ResetTaskEnvironment(ctx context.Context, taskID string, opts // On any non-idempotent error, the caller should preserve the row so the user // can retry. func (s *Service) teardownEnvironmentResources(ctx context.Context, env *models.TaskEnvironment) error { + return s.teardownEnvironmentResourcesWithWorktrees(ctx, env, true) +} + +func (s *Service) teardownEnvironmentRuntimeResources(ctx context.Context, env *models.TaskEnvironment) error { + return s.teardownEnvironmentResourcesWithWorktrees(ctx, env, false) +} + +func (s *Service) teardownEnvironmentResourcesWithWorktrees( + ctx context.Context, env *models.TaskEnvironment, includeWorktrees bool, +) error { if cause := context.Cause(ctx); cause != nil { return cause } worktreeIDs := environmentWorktreeIDs(env) - if env.ContainerID == "" && env.SandboxID == "" && len(worktreeIDs) == 0 { + if !environmentHasResources(env, includeWorktrees, worktreeIDs) { return nil } if s.envDestroyer == nil { @@ -415,14 +425,11 @@ func (s *Service) teardownEnvironmentResources(ctx context.Context, env *models. errs = append(errs, fmt.Errorf("destroy sandbox %s: %w", env.SandboxID, err)) } } - for _, worktreeID := range worktreeIDs { - if err := contextError(); err != nil { - return err - } - if err := s.envDestroyer.DestroyWorktree(ctx, worktreeID); err != nil { - if !errors.Is(err, worktree.ErrWorktreeNotFound) { - errs = append(errs, fmt.Errorf("destroy worktree %s: %w", worktreeID, err)) - } + if includeWorktrees { + var stopErr error + errs, stopErr = s.destroyEnvironmentWorktrees(ctx, worktreeIDs, errs) + if stopErr != nil { + return errors.Join(errors.Join(errs...), stopErr) } } if err := contextError(); err != nil { @@ -431,6 +438,25 @@ func (s *Service) teardownEnvironmentResources(ctx context.Context, env *models. return errors.Join(errs...) } +func environmentHasResources(env *models.TaskEnvironment, includeWorktrees bool, worktreeIDs []string) bool { + return env.ContainerID != "" || env.SandboxID != "" || (includeWorktrees && len(worktreeIDs) > 0) +} + +func (s *Service) destroyEnvironmentWorktrees( + ctx context.Context, worktreeIDs []string, errs []error, +) ([]error, error) { + for _, worktreeID := range worktreeIDs { + if cause := context.Cause(ctx); cause != nil { + return errs, cause + } + if err := s.envDestroyer.DestroyWorktree(ctx, worktreeID); err != nil && + !errors.Is(err, worktree.ErrWorktreeNotFound) { + errs = append(errs, fmt.Errorf("destroy worktree %s: %w", worktreeID, err)) + } + } + return errs, nil +} + // environmentWorktreeIDs returns the physical worktree identities recorded on // the environment's repository rows. func environmentWorktreeIDs(env *models.TaskEnvironment) []string { diff --git a/apps/backend/internal/task/service/service_task_environments_test.go b/apps/backend/internal/task/service/service_task_environments_test.go index ef789f5473..df6d3e9316 100644 --- a/apps/backend/internal/task/service/service_task_environments_test.go +++ b/apps/backend/internal/task/service/service_task_environments_test.go @@ -341,6 +341,33 @@ func TestCleanupTaskEnvironment_CancellationPreservesEnvironmentRow(t *testing.T } } +func TestCleanupDestructiveTaskResources_DoesNotDuplicateBatchWorktreeCleanup(t *testing.T) { + repo := &stubEnvRepo{env: &models.TaskEnvironment{ID: "env-1", TaskID: "task-1"}} + svc := newResetTestService(t, repo) + destroyer := &stubDestroyer{} + cleaner := &policyRecordingWorktreeCleanup{} + svc.SetEnvironmentDestroyer(destroyer) + svc.SetWorktreeCleanup(cleaner) + wt := &worktree.Worktree{ID: "wt-once", TaskID: "task-1"} + + errs := svc.cleanupDestructiveTaskResources( + context.Background(), "task-1", nil, []*worktree.Worktree{wt}, + taskEnvironmentCleanup{ + env: &models.TaskEnvironment{ID: "env-1", TaskID: "task-1", Repos: []*models.TaskEnvironmentRepo{{WorktreeID: wt.ID}}}, + preserveBranches: true, + }, nil, + ) + if len(errs) != 0 { + t.Fatalf("cleanup errors = %v", errs) + } + if len(destroyer.worktreeCalls) != 0 { + t.Fatalf("destroyer worktree calls = %v, want none", destroyer.worktreeCalls) + } + if cleaner.preservingCalls != 1 { + t.Fatalf("batch preserving calls = %d, want 1", cleaner.preservingCalls) + } +} + func TestResetTaskEnvironment_ContainerDestroyFailurePreservesRow(t *testing.T) { repo := &stubEnvRepo{env: &models.TaskEnvironment{ ID: "env-1", diff --git a/apps/backend/internal/task/service/service_tasks.go b/apps/backend/internal/task/service/service_tasks.go index 6c10728f32..301f9af42c 100644 --- a/apps/backend/internal/task/service/service_tasks.go +++ b/apps/backend/internal/task/service/service_tasks.go @@ -4088,12 +4088,36 @@ func (s *Service) cleanupTaskEnvironment( if !current { return nil } - if err := s.teardownEnvironmentResources(ctx, cleanup.env); err != nil { + _, archiveBatch := s.worktreeCleanup.(WorktreeArchiveBatchCleaner) + _, deleteBatch := s.worktreeCleanup.(WorktreeBatchCleaner) + needsArchiveBatch := false + if cleanup.env.ExecutorType == string(models.ExecutorTypeWorktree) { + for _, repo := range cleanup.env.Repos { + if repo != nil && repo.WorktreeID != "" && repo.Status != "deleted" { + needsArchiveBatch = true + break + } + } + } + if cleanup.preserveBranches && !archiveBatch && needsArchiveBatch { + if err := s.teardownEnvironmentRuntimeResources(ctx, cleanup.env); err != nil { + return []error{fmt.Errorf("teardown task environment %s runtime resources: %w", cleanup.env.ID, err)} + } + return []error{fmt.Errorf("archive cleanup requires a worktree archive batch cleaner")} + } + batchHandlesWorktrees := (cleanup.preserveBranches && archiveBatch) || (!cleanup.preserveBranches && deleteBatch) + var teardownErr error + if batchHandlesWorktrees { + teardownErr = s.teardownEnvironmentRuntimeResources(ctx, cleanup.env) + } else { + teardownErr = s.teardownEnvironmentResources(ctx, cleanup.env) + } + if teardownErr != nil { s.logger.Warn("failed to teardown task environment during task cleanup", zap.String("task_id", taskID), zap.String("env_id", cleanup.env.ID), - zap.Error(err)) - return []error{fmt.Errorf("teardown task environment %s: %w", cleanup.env.ID, err)} + zap.Error(teardownErr)) + return []error{fmt.Errorf("teardown task environment %s: %w", cleanup.env.ID, teardownErr)} } if cleanup.deleteRow { if cause := context.Cause(ctx); cause != nil { diff --git a/apps/backend/internal/worktree/branch_cleanup.go b/apps/backend/internal/worktree/branch_cleanup.go new file mode 100644 index 0000000000..a5a4f45094 --- /dev/null +++ b/apps/backend/internal/worktree/branch_cleanup.go @@ -0,0 +1,636 @@ +package worktree + +import ( + "context" + "crypto/sha256" + "errors" + "expvar" + "fmt" + "os/exec" + "regexp" + "sort" + "strings" + "time" + + "go.uber.org/zap" +) + +const ( + BranchOwnerUnknown = "unknown" + BranchOwnerManaged = "kandev" + BranchOwnerExternal = "external" +) + +type BranchRetentionReason string + +const ( + RetainedUnknownOwner BranchRetentionReason = "unknown_owner" + RetainedExternalOwner BranchRetentionReason = "external_owner" + RetainedAmbiguousOwner BranchRetentionReason = "ambiguous_owner" + RetainedActiveReference BranchRetentionReason = "active_reference" + RetainedMissingMetadata BranchRetentionReason = "missing_metadata" + RetainedLiveWorktree BranchRetentionReason = "live_worktree" + RetainedProtectedRef BranchRetentionReason = "protected_ref" + RetainedProtectedRefUnavailable BranchRetentionReason = "protected_ref_unavailable" + RetainedMissingRef BranchRetentionReason = "missing_ref" + RetainedAncestryProbeFailed BranchRetentionReason = "ancestry_probe_failed" + RetainedNotIntegrated BranchRetentionReason = "not_integrated" + RetainedRecoveryPersistFailed BranchRetentionReason = "recovery_persist_failed" + RetainedHeadChanged BranchRetentionReason = "head_changed" + RetainedSafeDeleteRefused BranchRetentionReason = "safe_delete_refused" + RetainedArchiveStateChanged BranchRetentionReason = "archive_state_changed" +) + +type BranchCleanupReceipt struct { + Attempted int `json:"attempted"` + Deleted int `json:"deleted"` + Retained int `json:"retained"` + RetainedReasons map[BranchRetentionReason]int `json:"retained_reasons"` +} + +var branchCleanupMetrics = expvar.NewMap("worktree_branch_cleanup_total") + +func createdBranchOwner(checkoutBranch, actualBranch string) string { + if checkoutBranch == "" || checkoutBranch != actualBranch { + return BranchOwnerManaged + } + return BranchOwnerExternal +} + +func newBranchCleanupReceipt() BranchCleanupReceipt { + return BranchCleanupReceipt{RetainedReasons: make(map[BranchRetentionReason]int)} +} + +func (r *BranchCleanupReceipt) merge(other BranchCleanupReceipt) { + r.Attempted += other.Attempted + r.Deleted += other.Deleted + r.Retained += other.Retained + if r.RetainedReasons == nil { + r.RetainedReasons = make(map[BranchRetentionReason]int) + } + for reason, count := range other.RetainedReasons { + r.RetainedReasons[reason] += count + } +} + +func (r *BranchCleanupReceipt) retain(reason BranchRetentionReason) { + r.Retained++ + r.RetainedReasons[reason]++ + branchCleanupMetrics.Add("retained:"+string(reason), 1) +} + +func (r BranchCleanupReceipt) reasonFields() []zap.Field { + reasons := make([]string, 0, len(r.RetainedReasons)) + for reason := range r.RetainedReasons { + reasons = append(reasons, string(reason)) + } + sort.Strings(reasons) + fields := make([]zap.Field, 0, len(reasons)+3) + fields = append(fields, + zap.Int("attempted", r.Attempted), + zap.Int("deleted", r.Deleted), + zap.Int("retained", r.Retained), + ) + for _, reason := range reasons { + fields = append(fields, zap.Int("retained_"+reason, r.RetainedReasons[BranchRetentionReason(reason)])) + } + return fields +} + +var commitSHA = regexp.MustCompile(`^[0-9a-fA-F]{40}([0-9a-fA-F]{24})?$`) + +func (m *Manager) compactManagedBranch(ctx context.Context, wt *Worktree) BranchCleanupReceipt { + return m.compactManagedBranchWithMode(ctx, wt, false) +} + +func (m *Manager) compactArchivedManagedBranch(ctx context.Context, wt *Worktree) BranchCleanupReceipt { + return m.compactManagedBranchWithMode(ctx, wt, true) +} + +func (m *Manager) compactManagedBranchWithMode( + ctx context.Context, wt *Worktree, requireArchived bool, +) BranchCleanupReceipt { + receipt := newBranchCleanupReceipt() + receipt.Attempted = 1 + branchCleanupMetrics.Add("attempted", 1) + + metadataStore, reason := m.branchMetadataStoreForCleanup(ctx, wt) + if reason != "" { + receipt.retain(reason) + return receipt + } + completed, reason := m.reconcileInterruptedArchivedCompaction(ctx, metadataStore, wt, requireArchived) + if reason != "" { + receipt.retain(reason) + return receipt + } + if completed { + receipt.Deleted = 1 + branchCleanupMetrics.Add("deleted", 1) + return receipt + } + branchHead, reason := m.verifiedIntegratedBranchHead(ctx, wt, requireArchived) + if reason != "" { + receipt.retain(reason) + return receipt + } + if reason = m.persistRecoveryAndDeleteBranch(ctx, metadataStore, wt, branchHead, requireArchived); reason != "" { + receipt.retain(reason) + return receipt + } + + receipt.Deleted = 1 + branchCleanupMetrics.Add("deleted", 1) + return receipt +} + +func (m *Manager) branchMetadataStoreForCleanup( + ctx context.Context, wt *Worktree, +) (BranchMetadataStore, BranchRetentionReason) { + switch wt.BranchOwner { + case BranchOwnerManaged: + case BranchOwnerExternal: + return nil, RetainedExternalOwner + default: + return nil, RetainedUnknownOwner + } + if strings.TrimSpace(wt.ID) == "" || strings.TrimSpace(wt.RepositoryID) == "" || + strings.TrimSpace(wt.RepositoryPath) == "" || strings.TrimSpace(wt.Branch) == "" || + strings.TrimSpace(wt.IntegrationRef) == "" { + return nil, RetainedMissingMetadata + } + metadataStore, ok := m.store.(BranchMetadataStore) + if !ok { + return nil, RetainedMissingMetadata + } + owners, err := metadataStore.CountWorktreeBranchOwners(ctx, wt.RepositoryPath, wt.Branch) + if err != nil || owners != 1 { + return nil, RetainedAmbiguousOwner + } + return metadataStore, "" +} + +func (m *Manager) verifiedIntegratedBranchHead( + ctx context.Context, wt *Worktree, requireArchived bool, +) (string, BranchRetentionReason) { + branchRef := "refs/heads/" + wt.Branch + live, err := m.branchCheckedOutInWorktree(ctx, wt.RepositoryPath, branchRef) + if err != nil { + return "", RetainedAncestryProbeFailed + } + if live { + return "", RetainedLiveWorktree + } + if protectedBranchName(wt.Branch, wt.BaseBranch) || + protectedBranchName(wt.Branch, wt.IntegrationRef) { + return "", RetainedProtectedRef + } + protected, reason := m.protectedRepositoryDefaultBranch(ctx, wt) + if reason != "" { + return "", reason + } + if protected { + return "", RetainedProtectedRef + } + + branchHead, err := m.resolveCommit(ctx, wt.RepositoryPath, branchRef) + if err != nil { + return "", RetainedMissingRef + } + if reason := m.verifyHeadAgainstIntegration(ctx, wt, branchHead); reason != "" { + return "", reason + } + return branchHead, "" +} + +// protectedRepositoryDefaultBranch loads the canonical default branch from +// persisted repository metadata. Worktree BaseBranch and IntegrationRef may +// differ from the canonical default branch, so every inferred-safe compaction +// must prove this protection independently. The lookup is intentionally exact +// and never derives a protected ref from the candidate branch name or a remote +// short name. +func (m *Manager) protectedRepositoryDefaultBranch( + ctx context.Context, wt *Worktree, +) (bool, BranchRetentionReason) { + if m.repoProvider == nil || wt == nil || strings.TrimSpace(wt.RepositoryID) == "" { + return false, RetainedProtectedRefUnavailable + } + repo, err := m.repoProvider.GetRepository(ctx, wt.RepositoryID) + if err != nil || repo == nil || strings.TrimSpace(repo.DefaultBranch) == "" { + if err != nil { + m.logger.Debug("repository default branch unavailable during cleanup", zap.Error(err)) + } + return false, RetainedProtectedRefUnavailable + } + return protectedBranchName(wt.Branch, repo.DefaultBranch), "" +} + +func (m *Manager) verifyHeadAgainstIntegration( + ctx context.Context, wt *Worktree, branchHead string, +) BranchRetentionReason { + integrationRef := normalizeIntegrationRef(wt.IntegrationRef) + integrationHead, err := m.resolveCommit(ctx, wt.RepositoryPath, integrationRef) + if err != nil { + return RetainedMissingRef + } + integrated, err := m.refContains(ctx, wt.RepositoryPath, integrationHead, branchHead) + if err != nil { + return RetainedAncestryProbeFailed + } + if !integrated { + return RetainedNotIntegrated + } + return "" +} + +func (m *Manager) persistRecoveryAndDeleteBranch( + ctx context.Context, + metadataStore BranchMetadataStore, + wt *Worktree, + branchHead string, + requireArchived bool, +) BranchRetentionReason { + persisted, reason := m.persistBranchRecoveryHead( + ctx, metadataStore, wt, branchHead, requireArchived, + ) + if reason != "" { + return reason + } + wt.RecoveryHeadSHA = branchHead + if reason = m.createRecoveryRef(ctx, wt, branchHead); reason != "" { + // The recovery ref may belong to a conflicting claimant, so do not try + // to delete it. Roll back only this row's just-persisted SHA. + if cleared, err := metadataStore.PersistBranchRecoveryHead(ctx, wt.ID, branchHead, ""); err == nil && cleared { + wt.RecoveryHeadSHA = "" + } + return reason + } + if requireArchived && !m.archivedBranchMutationStillAllowed(ctx, metadataStore, wt, branchHead) { + return RetainedArchiveStateChanged + } + if !persisted { + return RetainedRecoveryPersistFailed + } + reason = m.deleteExpectedBranchRef(ctx, metadataStore, wt, branchHead) + if reason == "" { + reason = m.recordBranchCompactionComplete(ctx, metadataStore, wt, branchHead, requireArchived) + } + return reason +} + +func (m *Manager) persistBranchRecoveryHead( + ctx context.Context, + metadataStore BranchMetadataStore, + wt *Worktree, + branchHead string, + requireArchived bool, +) (bool, BranchRetentionReason) { + if requireArchived { + maintenanceStore, ok := m.store.(ArchivedBranchMaintenanceStore) + if !ok { + return false, RetainedArchiveStateChanged + } + persisted, err := maintenanceStore.PersistArchivedBranchRecoveryHead( + ctx, wt.ID, wt.RecoveryHeadSHA, branchHead, + ) + if err == nil && !persisted { + return false, RetainedArchiveStateChanged + } + if err != nil { + return false, RetainedRecoveryPersistFailed + } + return true, "" + } + persisted, err := metadataStore.PersistBranchRecoveryHead(ctx, wt.ID, wt.RecoveryHeadSHA, branchHead) + if err != nil || !persisted { + return false, RetainedRecoveryPersistFailed + } + return true, "" +} + +func (m *Manager) archivedBranchMutationStillAllowed( + ctx context.Context, metadataStore BranchMetadataStore, wt *Worktree, branchHead string, +) bool { + maintenanceStore, ok := m.store.(ArchivedBranchMaintenanceStore) + if !ok { + return false + } + eligible, err := maintenanceStore.IsArchivedBranchCandidate(ctx, wt.ID) + if err == nil && eligible { + return true + } + m.clearBranchRecoveryHead(ctx, metadataStore, wt, branchHead) + return false +} + +func (m *Manager) deleteExpectedBranchRef( + ctx context.Context, metadataStore BranchMetadataStore, wt *Worktree, branchHead string, +) BranchRetentionReason { + branchRef := "refs/heads/" + wt.Branch + live, err := m.branchCheckedOutInWorktree(ctx, wt.RepositoryPath, branchRef) + if err != nil { + m.clearBranchRecoveryHead(ctx, metadataStore, wt, branchHead) + return RetainedAncestryProbeFailed + } + if live { + m.clearBranchRecoveryHead(ctx, metadataStore, wt, branchHead) + return RetainedLiveWorktree + } + // Recheck immediately before the destructive ref mutation. A worktree can + // be attached after the initial eligibility probe without changing the + // branch head, and update-ref does not enforce Git's checked-out-branch + // protection. + live, err = m.branchCheckedOutInWorktree(ctx, wt.RepositoryPath, branchRef) + if err != nil { + m.clearBranchRecoveryHead(ctx, metadataStore, wt, branchHead) + return RetainedAncestryProbeFailed + } + if live { + m.clearBranchRecoveryHead(ctx, metadataStore, wt, branchHead) + return RetainedLiveWorktree + } + mutationCtx, cancel := context.WithTimeout(ctx, m.inspectTimeout) + defer cancel() + cmd := m.newNonInteractiveGitCmd(mutationCtx, wt.RepositoryPath, "update-ref", "-d", branchRef, branchHead) + output, err := runGitCmdCombinedOutput(mutationCtx, cmd) + if err == nil { + return m.verifyDeletionDidNotRaceLiveness(ctx, metadataStore, wt, branchRef, branchHead) + } + currentHead, resolveErr := m.resolveCommit(ctx, wt.RepositoryPath, branchRef) + if resolveErr == nil { + m.clearBranchRecoveryHead(ctx, metadataStore, wt, branchHead) + if currentHead != branchHead { + return RetainedHeadChanged + } + } + m.logger.Debug("exact managed branch deletion refused", zap.String("output", strings.TrimSpace(string(output)))) + return RetainedSafeDeleteRefused +} + +func (m *Manager) verifyDeletionDidNotRaceLiveness( + ctx context.Context, + metadataStore BranchMetadataStore, + wt *Worktree, + branchRef string, + branchHead string, +) BranchRetentionReason { + safetyCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), m.inspectTimeout) + defer cancel() + live, err := m.branchCheckedOutInWorktree(safetyCtx, wt.RepositoryPath, branchRef) + if err == nil && !live { + return "" + } + restoreReason := m.restoreDeletedBranchRef(safetyCtx, wt, branchRef, branchHead) + if restoreReason != "" { + return restoreReason + } + m.clearBranchRecoveryHead(safetyCtx, metadataStore, wt, branchHead) + if err != nil { + return RetainedAncestryProbeFailed + } + return RetainedLiveWorktree +} + +func (m *Manager) restoreDeletedBranchRef( + ctx context.Context, wt *Worktree, branchRef, branchHead string, +) BranchRetentionReason { + zeroOID := strings.Repeat("0", len(branchHead)) + cmd := m.newNonInteractiveGitCmd(ctx, wt.RepositoryPath, "update-ref", branchRef, branchHead, zeroOID) + if _, err := runGitCmdCombinedOutput(ctx, cmd); err == nil { + return "" + } + currentHead, err := m.resolveCommit(ctx, wt.RepositoryPath, branchRef) + if err != nil { + return RetainedSafeDeleteRefused + } + if currentHead != branchHead { + return RetainedHeadChanged + } + return "" +} + +func (m *Manager) recordBranchCompactionComplete( + ctx context.Context, + metadataStore BranchMetadataStore, + wt *Worktree, + branchHead string, + requireArchived bool, +) BranchRetentionReason { + var persisted bool + var err error + if requireArchived { + maintenanceStore, ok := m.store.(ArchivedBranchMaintenanceStore) + if ok { + persisted, err = maintenanceStore.PersistArchivedBranchCompactionComplete(ctx, wt.ID, branchHead) + } + } else { + persisted, err = metadataStore.PersistBranchCompactionComplete(ctx, wt.ID, branchHead) + } + if err != nil || !persisted { + m.logger.Warn("managed branch deletion completion was not persisted", zap.Error(err)) + if requireArchived && err == nil { + branchRef := "refs/heads/" + wt.Branch + if reason := m.restoreDeletedBranchRef(ctx, wt, branchRef, branchHead); reason != "" { + return reason + } + m.clearBranchRecoveryHead(ctx, metadataStore, wt, branchHead) + return RetainedArchiveStateChanged + } + return "" + } + now := time.Now().UTC() + wt.BranchCompactedAt = &now + return "" +} + +func (m *Manager) reconcileInterruptedArchivedCompaction( + ctx context.Context, + metadataStore BranchMetadataStore, + wt *Worktree, + requireArchived bool, +) (bool, BranchRetentionReason) { + if !requireArchived || strings.TrimSpace(wt.RecoveryHeadSHA) == "" { + return false, "" + } + branchRef := "refs/heads/" + wt.Branch + exists, err := m.localBranchRefExists(ctx, wt.RepositoryPath, branchRef) + if err != nil { + return true, RetainedAncestryProbeFailed + } + if exists { + currentHead, resolveErr := m.resolveCommit(ctx, wt.RepositoryPath, branchRef) + if resolveErr != nil { + return true, RetainedMissingRef + } + if !strings.EqualFold(currentHead, wt.RecoveryHeadSHA) { + // A local ref may have advanced after the recovery head was + // durably recorded. Retain it and never replace the recovery SHA. + return true, RetainedHeadChanged + } + return false, "" + } + return m.completeInterruptedArchivedCompaction(ctx, metadataStore, wt, branchRef) +} + +func (m *Manager) completeInterruptedArchivedCompaction( + ctx context.Context, + metadataStore BranchMetadataStore, + wt *Worktree, + branchRef string, +) (bool, BranchRetentionReason) { + live, err := m.branchCheckedOutInWorktree(ctx, wt.RepositoryPath, branchRef) + if err != nil { + return true, RetainedAncestryProbeFailed + } + if live { + reason := m.restoreDeletedBranchRef(ctx, wt, branchRef, wt.RecoveryHeadSHA) + if reason == "" { + m.clearBranchRecoveryHead(ctx, metadataStore, wt, wt.RecoveryHeadSHA) + reason = RetainedLiveWorktree + } + return true, reason + } + branchHead, reason := m.interruptedRecoveryHeadIsSafe(ctx, wt) + if reason != "" { + return true, reason + } + maintenanceStore, ok := m.store.(ArchivedBranchMaintenanceStore) + if !ok { + return true, RetainedArchiveStateChanged + } + persisted, err := maintenanceStore.PersistArchivedBranchCompactionComplete(ctx, wt.ID, branchHead) + if err != nil { + return true, RetainedRecoveryPersistFailed + } + if !persisted { + return true, RetainedArchiveStateChanged + } + return true, "" +} + +func (m *Manager) interruptedRecoveryHeadIsSafe( + ctx context.Context, wt *Worktree, +) (string, BranchRetentionReason) { + if protectedBranchName(wt.Branch, wt.BaseBranch) || protectedBranchName(wt.Branch, wt.IntegrationRef) { + return "", RetainedProtectedRef + } + protected, reason := m.protectedRepositoryDefaultBranch(ctx, wt) + if reason != "" { + return "", reason + } + if protected { + return "", RetainedProtectedRef + } + branchHead, err := m.resolveCommit(ctx, wt.RepositoryPath, wt.RecoveryHeadSHA) + if err != nil || branchHead != strings.ToLower(wt.RecoveryHeadSHA) { + return "", RetainedMissingRef + } + recoveryHead, err := m.resolveCommit(ctx, wt.RepositoryPath, recoveryRefName(wt.ID)) + if err != nil || !strings.EqualFold(recoveryHead, branchHead) { + return "", RetainedMissingRef + } + if reason := m.verifyHeadAgainstIntegration(ctx, wt, branchHead); reason != "" { + return "", reason + } + return branchHead, "" +} + +func (m *Manager) localBranchRefExists(ctx context.Context, repoPath, branchRef string) (bool, error) { + _, err := m.runBoundedGitInspect(ctx, repoPath, "show-ref", "--verify", "--quiet", branchRef) + if err == nil { + return true, nil + } + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 { + return false, nil + } + return false, err +} + +func (m *Manager) clearBranchRecoveryHead( + ctx context.Context, metadataStore BranchMetadataStore, wt *Worktree, branchHead string, +) { + if !m.deleteRecoveryRef(ctx, wt, branchHead) { + return + } + cleared, err := metadataStore.PersistBranchRecoveryHead(ctx, wt.ID, branchHead, "") + if err == nil && cleared { + wt.RecoveryHeadSHA = "" + } +} + +// recoveryRefName is derived solely from the immutable worktree identity. It +// keeps a compacted task's exact commit reachable without exposing a branch +// name or accepting a caller-controlled ref path. +func recoveryRefName(worktreeID string) string { + sum := sha256.Sum256([]byte(worktreeID)) + return fmt.Sprintf("refs/kandev/recovery/%x", sum[:]) +} + +func (m *Manager) createRecoveryRef(ctx context.Context, wt *Worktree, branchHead string) BranchRetentionReason { + ref := recoveryRefName(wt.ID) + zeroOID := strings.Repeat("0", len(branchHead)) + cmd := m.newNonInteractiveGitCmd(ctx, wt.RepositoryPath, "update-ref", ref, branchHead, zeroOID) + if _, err := runGitCmdCombinedOutput(ctx, cmd); err == nil { + return "" + } + existing, err := m.resolveCommit(ctx, wt.RepositoryPath, ref) + if err == nil && strings.EqualFold(existing, branchHead) { + return "" + } + return RetainedRecoveryPersistFailed +} + +func (m *Manager) deleteRecoveryRef(ctx context.Context, wt *Worktree, branchHead string) bool { + ref := recoveryRefName(wt.ID) + cmd := m.newNonInteractiveGitCmd(ctx, wt.RepositoryPath, "update-ref", "-d", ref, branchHead) + if _, err := runGitCmdCombinedOutput(ctx, cmd); err == nil { + return true + } + exists, err := m.localBranchRefExists(ctx, wt.RepositoryPath, ref) + return err == nil && !exists +} + +func protectedBranchName(branch, protectedRef string) bool { + protectedRef = strings.TrimSpace(protectedRef) + protectedRef = strings.TrimPrefix(protectedRef, "refs/heads/") + protectedRef = strings.TrimPrefix(protectedRef, "refs/remotes/origin/") + protectedRef = strings.TrimPrefix(protectedRef, "origin/") + return protectedRef != "" && branch == protectedRef +} + +func normalizeIntegrationRef(ref string) string { + ref = strings.TrimSpace(ref) + if strings.HasPrefix(ref, "refs/") { + return ref + } + if strings.HasPrefix(ref, "origin/") { + return "refs/remotes/" + ref + } + return "refs/heads/" + ref +} + +func (m *Manager) branchCheckedOutInWorktree(ctx context.Context, repoPath, branchRef string) (bool, error) { + output, err := m.runBoundedGitInspect(ctx, repoPath, "worktree", "list", "--porcelain", "-z") + if err != nil { + return false, err + } + for _, registration := range parseWorktreeRegistrations(output) { + if registration.branch == branchRef { + return true, nil + } + } + return false, nil +} + +func (m *Manager) resolveCommit(ctx context.Context, repoPath, ref string) (string, error) { + output, err := m.runBoundedGitInspect(ctx, repoPath, "rev-parse", "--verify", ref+"^{commit}") + if err != nil { + return "", err + } + sha := strings.TrimSpace(output) + if !commitSHA.MatchString(sha) { + return "", fmt.Errorf("resolved invalid commit object") + } + return strings.ToLower(sha), nil +} diff --git a/apps/backend/internal/worktree/branch_cleanup_interruption_test.go b/apps/backend/internal/worktree/branch_cleanup_interruption_test.go new file mode 100644 index 0000000000..f95321abc0 --- /dev/null +++ b/apps/backend/internal/worktree/branch_cleanup_interruption_test.go @@ -0,0 +1,498 @@ +package worktree + +import ( + "context" + "errors" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + "testing" + + "github.com/kandev/kandev/internal/task/models" +) + +type cancelAfterArchivedRecoveryStore struct { + *SQLiteStore + cancel context.CancelFunc + once sync.Once +} + +type failOnceArchivedCompactionCompleteStore struct { + *SQLiteStore + failed bool +} + +type failOnceBranchRecoveryRestoredStore struct { + *SQLiteStore + failed bool +} + +type panicArchivedCandidateStore struct { + *SQLiteStore +} + +func (s *panicArchivedCandidateStore) IsArchivedBranchCandidate(context.Context, string) (bool, error) { + panic("injected archived candidate panic") +} + +func (s *failOnceArchivedCompactionCompleteStore) PersistArchivedBranchCompactionComplete( + ctx context.Context, worktreeID, expectedRecoveryHead string, +) (bool, error) { + if !s.failed { + s.failed = true + return false, errors.New("injected compaction completion failure") + } + return s.SQLiteStore.PersistArchivedBranchCompactionComplete(ctx, worktreeID, expectedRecoveryHead) +} + +func (s *failOnceBranchRecoveryRestoredStore) PersistBranchRecoveryRestored( + ctx context.Context, worktreeID, expectedRecoveryHead string, +) (bool, error) { + if !s.failed { + s.failed = true + return false, errors.New("injected recovery finalization failure") + } + return s.SQLiteStore.PersistBranchRecoveryRestored(ctx, worktreeID, expectedRecoveryHead) +} + +func (s *cancelAfterArchivedRecoveryStore) PersistArchivedBranchRecoveryHead( + ctx context.Context, worktreeID, expected, recoveryHead string, +) (bool, error) { + persisted, err := s.SQLiteStore.PersistArchivedBranchRecoveryHead( + ctx, worktreeID, expected, recoveryHead, + ) + if err == nil && persisted { + s.once.Do(s.cancel) + } + return persisted, err +} + +func TestMaintainArchivedBranches_RetriesAfterRecoveryPersistInterruption(t *testing.T) { + mgr, store, wt, wantHead := archivedIntegratedBranchForMaintenance(t, "interrupted") + + interruptedCtx, cancel := context.WithCancel(context.Background()) + mgr.store = &cancelAfterArchivedRecoveryStore{SQLiteStore: store, cancel: cancel} + firstReceipt, firstErr := mgr.MaintainArchivedBranches(interruptedCtx, 1) + if firstErr == nil { + t.Fatal("interrupted maintenance error = nil, want context cancellation") + } + if firstReceipt.Deleted != 0 { + t.Fatalf("interrupted maintenance receipt = %+v, want no deletion", firstReceipt) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "rev-parse", wt.Branch)); got != wantHead { + t.Fatalf("branch head after interruption = %q, want %q", got, wantHead) + } + + mgr.store = store + secondReceipt, err := mgr.MaintainArchivedBranches(context.Background(), 1) + if err != nil { + t.Fatalf("retry archived branch maintenance: %v", err) + } + if secondReceipt.Attempted != 1 || secondReceipt.Deleted != 1 { + t.Fatalf("retry receipt = %+v, want one attempted deletion", secondReceipt) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "branch", "--list", wt.Branch)); got != "" { + t.Fatalf("integrated archived branch remains after retry: %q", got) + } + thirdReceipt, err := mgr.MaintainArchivedBranches(context.Background(), 1) + if err != nil { + t.Fatalf("maintenance after completed compaction: %v", err) + } + if thirdReceipt.Attempted != 0 { + t.Fatalf("completed branch was selected again: %+v", thirdReceipt) + } +} + +func TestMaintainArchivedBranches_KeepsCompactedRecoveryReachableThroughGC(t *testing.T) { + mgr, store, wt, wantHead := archivedIntegratedBranchForMaintenance(t, "gc-recovery") + receipt, err := mgr.MaintainArchivedBranches(context.Background(), 1) + if err != nil || receipt.Deleted != 1 { + t.Fatalf("compact archived branch: receipt=%+v err=%v", receipt, err) + } + runGit(t, wt.RepositoryPath, "update-ref", "refs/heads/main", strings.Repeat("0", 40)) + runGit(t, wt.RepositoryPath, "reflog", "expire", "--expire=now", "--all") + runGit(t, wt.RepositoryPath, "gc", "--prune=now") + persisted, err := store.GetWorktreeByID(context.Background(), wt.ID) + if err != nil { + t.Fatalf("load compacted recovery metadata: %v", err) + } + if err := mgr.restoreManagedBranchFromRecoveryHeadLocked(context.Background(), persisted); err != nil { + t.Fatalf("restore after integration ref rewrite and gc: %v", err) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "rev-parse", "refs/heads/"+wt.Branch)); got != wantHead { + t.Fatalf("restored head = %q, want %q", got, wantHead) + } + if output, err := exec.Command("git", "-C", wt.RepositoryPath, "show-ref", "--verify", "--quiet", recoveryRefName(wt.ID)).CombinedOutput(); err == nil { + t.Fatalf("recovery ref remains after successful restoration: %s", output) + } + if persisted, err := store.GetWorktreeByID(context.Background(), wt.ID); err != nil || persisted.BranchCompactedAt != nil { + t.Fatalf("restored compaction state = %+v err=%v, want uncompacted", persisted, err) + } +} + +func TestMaintainArchivedBranches_RearchivesRestoredBranchWithoutSessionLaunch(t *testing.T) { + mgr, store, wt, wantHead := archivedIntegratedBranchForMaintenance(t, "rearchive-restored") + first, err := mgr.MaintainArchivedBranches(context.Background(), 1) + if err != nil || first.Deleted != 1 { + t.Fatalf("initial compaction: receipt=%+v err=%v", first, err) + } + persisted, err := store.GetWorktreeByID(context.Background(), wt.ID) + if err != nil { + t.Fatalf("load compacted branch: %v", err) + } + if _, err := store.db.ExecContext(context.Background(), `UPDATE tasks SET archived_at = NULL WHERE id = ?`, wt.TaskID); err != nil { + t.Fatalf("unarchive task: %v", err) + } + if status := mgr.RecoverBranchStatus(context.Background(), persisted); status != BranchStatusLocal { + t.Fatalf("unarchive recovery status = %q, want local", status) + } + persisted, err = store.GetWorktreeByID(context.Background(), wt.ID) + if err != nil || persisted.BranchCompactedAt != nil || persisted.RecoveryHeadSHA != "" { + t.Fatalf("restored metadata = %+v err=%v, want uncompacted", persisted, err) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "rev-parse", "refs/heads/"+wt.Branch)); got != wantHead { + t.Fatalf("restored branch head = %q, want %q", got, wantHead) + } + if _, err := store.db.ExecContext(context.Background(), `UPDATE tasks SET archived_at = CURRENT_TIMESTAMP WHERE id = ?`, wt.TaskID); err != nil { + t.Fatalf("rearchive task: %v", err) + } + + second, err := mgr.MaintainArchivedBranches(context.Background(), 1) + if err != nil || second.Attempted != 1 || second.Deleted != 1 { + t.Fatalf("rearchive maintenance: receipt=%+v err=%v", second, err) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "branch", "--list", wt.Branch)); got != "" { + t.Fatalf("rearchived integrated branch remains: %q", got) + } +} + +func TestRecoverBranchStatus_FinalizesInterruptedPreDeleteRecovery(t *testing.T) { + mgr, store, wt, wantHead := archivedIntegratedBranchForMaintenance(t, "recover-before-delete") + ctx := context.Background() + if persisted, err := store.PersistArchivedBranchRecoveryHead(ctx, wt.ID, "", wantHead); err != nil || !persisted { + t.Fatalf("persist recovery head: persisted=%v err=%v", persisted, err) + } + if reason := mgr.createRecoveryRef(ctx, wt, wantHead); reason != "" { + t.Fatalf("create recovery ref: %s", reason) + } + if _, err := store.db.ExecContext(ctx, `UPDATE tasks SET archived_at = NULL WHERE id = ?`, wt.TaskID); err != nil { + t.Fatalf("unarchive task: %v", err) + } + persisted, err := store.GetWorktreeByID(ctx, wt.ID) + if err != nil { + t.Fatalf("load interrupted recovery: %v", err) + } + if persisted.BranchCompactedAt != nil { + t.Fatalf("interrupted recovery has compaction marker: %v", persisted.BranchCompactedAt) + } + + if status := mgr.RecoverBranchStatus(ctx, persisted); status != BranchStatusLocal { + t.Fatalf("recovery status = %q, want local", status) + } + restored, err := store.GetWorktreeByID(ctx, wt.ID) + if err != nil { + t.Fatalf("load finalized recovery: %v", err) + } + if restored.RecoveryHeadSHA != "" || restored.BranchCompactedAt != nil { + t.Fatalf("finalized recovery metadata = head %q compacted %v, want cleared", + restored.RecoveryHeadSHA, restored.BranchCompactedAt) + } + if exists, err := mgr.localBranchRefExists(ctx, wt.RepositoryPath, recoveryRefName(wt.ID)); err != nil || exists { + t.Fatalf("recovery ref after finalization: exists=%v err=%v, want absent", exists, err) + } +} + +func TestRecoverBranchStatus_RetainsMetadataWhenRecoveryRefDeleteFails(t *testing.T) { + mgr, store, wt, wantHead := archivedIntegratedBranchForMaintenance(t, "recover-delete-failure") + ctx := context.Background() + receipt, err := mgr.MaintainArchivedBranches(ctx, 1) + if err != nil || receipt.Deleted != 1 { + t.Fatalf("compact archived branch: receipt=%+v err=%v", receipt, err) + } + persisted, err := store.GetWorktreeByID(ctx, wt.ID) + if err != nil { + t.Fatalf("load compacted branch: %v", err) + } + zeroOID := strings.Repeat("0", len(wantHead)) + runGit(t, wt.RepositoryPath, "update-ref", "refs/heads/"+wt.Branch, wantHead, zeroOID) + rejectRecoveryRefDelete(t, wt) + + if status := mgr.RecoverBranchStatus(ctx, persisted); status != BranchStatusMissing { + t.Fatalf("recovery status = %q, want missing when cleanup is incomplete", status) + } + retained, err := store.GetWorktreeByID(ctx, wt.ID) + if err != nil { + t.Fatalf("load retained recovery metadata: %v", err) + } + if retained.RecoveryHeadSHA != wantHead || retained.BranchCompactedAt == nil { + t.Fatalf("retained recovery metadata = head %q compacted %v, want head %q and marker", + retained.RecoveryHeadSHA, retained.BranchCompactedAt, wantHead) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "rev-parse", recoveryRefName(wt.ID))); got != wantHead { + t.Fatalf("recovery ref head = %q, want %q", got, wantHead) + } +} + +// Reviewer-requested contract coverage: a failure after the recovery ref is +// removed must leave the local branch and durable metadata retryable. +func TestRecoverBranchStatus_RetriesAfterRecoveryMetadataClearFails(t *testing.T) { + mgr, store, wt, wantHead := archivedIntegratedBranchForMaintenance(t, "recover-finalize-failure") + ctx := context.Background() + receipt, err := mgr.MaintainArchivedBranches(ctx, 1) + if err != nil || receipt.Deleted != 1 { + t.Fatalf("compact archived branch: receipt=%+v err=%v", receipt, err) + } + persisted, err := store.GetWorktreeByID(ctx, wt.ID) + if err != nil { + t.Fatalf("load compacted branch: %v", err) + } + + mgr.store = &failOnceBranchRecoveryRestoredStore{SQLiteStore: store} + if status := mgr.RecoverBranchStatus(ctx, persisted); status != BranchStatusLocal { + t.Fatalf("recovery status after metadata failure = %q, want local", status) + } + interrupted, err := store.GetWorktreeByID(ctx, wt.ID) + if err != nil { + t.Fatalf("load interrupted recovery: %v", err) + } + if interrupted.RecoveryHeadSHA != wantHead || interrupted.BranchCompactedAt == nil { + t.Fatalf("interrupted recovery metadata = head %q compacted %v, want head %q and marker", + interrupted.RecoveryHeadSHA, interrupted.BranchCompactedAt, wantHead) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "rev-parse", "refs/heads/"+wt.Branch)); got != wantHead { + t.Fatalf("restored branch head = %q, want %q", got, wantHead) + } + if exists, err := mgr.localBranchRefExists(ctx, wt.RepositoryPath, recoveryRefName(wt.ID)); err != nil || exists { + t.Fatalf("recovery ref after interrupted finalization: exists=%v err=%v, want absent", exists, err) + } + + mgr.store = store + if status := mgr.RecoverBranchStatus(ctx, interrupted); status != BranchStatusLocal { + t.Fatalf("recovery status after retry = %q, want local", status) + } + finalized, err := store.GetWorktreeByID(ctx, wt.ID) + if err != nil { + t.Fatalf("load finalized recovery: %v", err) + } + if finalized.RecoveryHeadSHA != "" || finalized.BranchCompactedAt != nil { + t.Fatalf("finalized recovery metadata = head %q compacted %v, want cleared", + finalized.RecoveryHeadSHA, finalized.BranchCompactedAt) + } +} + +// Reviewer-requested contract coverage: production already reconciles an +// absent ref whose recovery SHA persisted before its completion marker. +func TestMaintainArchivedBranches_FinalizesAbsentRefAfterCompletionPersistFailure(t *testing.T) { + mgr, store, wt, wantHead := archivedIntegratedBranchForMaintenance(t, "completion-interrupted") + mgr.store = &failOnceArchivedCompactionCompleteStore{SQLiteStore: store} + + firstReceipt, err := mgr.MaintainArchivedBranches(context.Background(), 1) + if err != nil { + t.Fatalf("maintenance with completion persistence failure: %v", err) + } + if firstReceipt.Attempted != 1 || firstReceipt.Deleted != 1 { + t.Fatalf("first receipt = %+v, want completed ref deletion", firstReceipt) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "branch", "--list", wt.Branch)); got != "" { + t.Fatalf("branch remains after exact-ref deletion: %q", got) + } + persisted, err := store.GetWorktreeByID(context.Background(), wt.ID) + if err != nil { + t.Fatalf("load interrupted compaction: %v", err) + } + if persisted.RecoveryHeadSHA != wantHead || persisted.BranchCompactedAt != nil { + t.Fatalf("interrupted metadata = head %q compacted %v, want head %q without marker", + persisted.RecoveryHeadSHA, persisted.BranchCompactedAt, wantHead) + } + + rejectBranchDeleteDuringRetry(t, wt) + mgr.store = store + secondReceipt, err := mgr.MaintainArchivedBranches(context.Background(), 1) + if err != nil { + t.Fatalf("finalize interrupted compaction: %v", err) + } + if secondReceipt.Attempted != 1 || secondReceipt.Deleted != 1 { + t.Fatalf("finalization receipt = %+v, want one finalized candidate", secondReceipt) + } + persisted, err = store.GetWorktreeByID(context.Background(), wt.ID) + if err != nil { + t.Fatalf("load finalized compaction: %v", err) + } + if persisted.RecoveryHeadSHA != wantHead || persisted.BranchCompactedAt == nil { + t.Fatalf("finalized metadata = head %q compacted %v, want preserved head %q and marker", + persisted.RecoveryHeadSHA, persisted.BranchCompactedAt, wantHead) + } + thirdReceipt, err := mgr.MaintainArchivedBranches(context.Background(), 1) + if err != nil { + t.Fatalf("maintenance after finalization: %v", err) + } + if thirdReceipt.Attempted != 0 { + t.Fatalf("finalized candidate selected again: %+v", thirdReceipt) + } +} + +func TestMaintainArchivedBranches_RetainsAdvancedRefAfterInterruptedCompaction(t *testing.T) { + mgr, store, wt, recoveryHead := archivedIntegratedBranchForMaintenance(t, "advanced-ref") + ctx := context.Background() + if persisted, err := store.PersistArchivedBranchRecoveryHead(ctx, wt.ID, "", recoveryHead); err != nil || !persisted { + t.Fatalf("persist recovery head: persisted=%v err=%v", persisted, err) + } + runGit(t, wt.RepositoryPath, "checkout", "main") + runGit(t, wt.RepositoryPath, "commit", "--allow-empty", "-m", "advance integration") + advancedHead := strings.TrimSpace(runGit(t, wt.RepositoryPath, "rev-parse", "HEAD")) + runGit(t, wt.RepositoryPath, "update-ref", "refs/heads/"+wt.Branch, advancedHead) + + receipt, err := mgr.MaintainArchivedBranches(ctx, 1) + if err != nil { + t.Fatalf("maintenance: %v", err) + } + if receipt.Deleted != 0 || receipt.RetainedReasons[RetainedHeadChanged] != 1 { + t.Fatalf("advanced-ref receipt = %+v", receipt) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "rev-parse", wt.Branch)); got != advancedHead { + t.Fatalf("advanced branch head = %q, want %q", got, advancedHead) + } + persisted, err := store.GetWorktreeByID(ctx, wt.ID) + if err != nil { + t.Fatalf("load recovery metadata: %v", err) + } + if persisted.RecoveryHeadSHA != recoveryHead { + t.Fatalf("recovery head overwritten = %q, want %q", persisted.RecoveryHeadSHA, recoveryHead) + } +} + +func TestMaintainArchivedBranches_ReleasesRepoLockAfterPanic(t *testing.T) { + mgr, store, wt, _ := archivedIntegratedBranchForMaintenance(t, "panic-lock") + mgr.store = &panicArchivedCandidateStore{SQLiteStore: store} + + var recovered any + func() { + defer func() { recovered = recover() }() + _, _ = mgr.MaintainArchivedBranches(context.Background(), 1) + }() + if recovered == nil { + t.Fatal("maintenance panic was not observed") + } + + repoLock := mgr.getRepoLock(wt.RepositoryPath) + if !repoLock.TryLock() { + t.Fatal("repository lock remained held after maintenance panic") + } + repoLock.Unlock() + mgr.releaseRepoLock(wt.RepositoryPath) +} + +func rejectBranchDeleteDuringRetry(t *testing.T, wt *Worktree) { + t.Helper() + realGit, err := exec.LookPath("git") + if err != nil { + t.Fatalf("find git: %v", err) + } + binDir := t.TempDir() + wrapper := filepath.Join(binDir, "git") + branchRef := "refs/heads/" + wt.Branch + script := "#!/bin/sh\n" + + "case \" $* \" in\n" + + " *\" update-ref -d " + branchRef + " \"*) exit 97 ;;\n" + + "esac\n" + + "exec \"" + realGit + "\" \"$@\"\n" + if err := os.WriteFile(wrapper, []byte(script), 0o755); err != nil { + t.Fatalf("write git wrapper: %v", err) + } + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) +} + +func rejectRecoveryRefDelete(t *testing.T, wt *Worktree) { + t.Helper() + realGit, err := exec.LookPath("git") + if err != nil { + t.Fatalf("find git: %v", err) + } + binDir := t.TempDir() + wrapper := filepath.Join(binDir, "git") + recoveryRef := recoveryRefName(wt.ID) + script := "#!/bin/sh\n" + + "case \" $* \" in\n" + + " *\" update-ref -d " + recoveryRef + " \"*) exit 97 ;;\n" + + "esac\n" + + "exec \"" + realGit + "\" \"$@\"\n" + if err := os.WriteFile(wrapper, []byte(script), 0o755); err != nil { + t.Fatalf("write git wrapper: %v", err) + } + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) +} + +func TestMaintainArchivedBranches_RestoresRefWhenBranchBecomesLiveDuringDelete(t *testing.T) { + mgr, _, wt, wantHead := archivedIntegratedBranchForMaintenance(t, "live-race") + + realGit, err := exec.LookPath("git") + if err != nil { + t.Fatalf("find git: %v", err) + } + binDir := t.TempDir() + racePath := filepath.Join(t.TempDir(), "racing-worktree") + wrapper := filepath.Join(binDir, "git") + branchRef := "refs/heads/" + wt.Branch + script := "#!/bin/sh\n" + + "case \" $* \" in\n" + + " *\" update-ref -d " + branchRef + " \"*)\n" + + " \"" + realGit + "\" -C \"" + wt.RepositoryPath + "\" worktree add \"" + racePath + "\" \"" + wt.Branch + "\" >/dev/null\n" + + " ;;\n" + + "esac\n" + + "exec \"" + realGit + "\" \"$@\"\n" + if err := os.WriteFile(wrapper, []byte(script), 0o755); err != nil { + t.Fatalf("write git wrapper: %v", err) + } + t.Cleanup(func() { + _ = exec.Command(realGit, "-C", wt.RepositoryPath, "worktree", "remove", "--force", racePath).Run() + }) + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + receipt, err := mgr.MaintainArchivedBranches(context.Background(), 1) + if err != nil { + t.Fatalf("maintenance with liveness race: %v", err) + } + if receipt.Deleted != 0 || receipt.RetainedReasons[RetainedLiveWorktree] != 1 { + t.Fatalf("liveness-race receipt = %+v, want live-worktree retention", receipt) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "rev-parse", branchRef)); got != wantHead { + t.Fatalf("branch head after liveness race = %q, want restored %q", got, wantHead) + } +} + +func archivedIntegratedBranchForMaintenance( + t *testing.T, suffix string, +) (*Manager, *SQLiteStore, *Worktree, string) { + t.Helper() + mgr, store := newReferenceCleanupTestManager(t) + ctx := context.Background() + taskID := "task-maintenance-" + suffix + sessionID := "session-maintenance-" + suffix + seedReferenceCleanupSession(t, store, taskID, sessionID, models.TaskSessionStateCompleted) + wt := createReferenceCleanupWorktree(t, mgr, taskID, sessionID) + if _, err := store.db.ExecContext(ctx, ` + INSERT INTO repositories (id, workspace_id, name, local_path, created_at, updated_at) + VALUES (?, 'workspace', 'repository', ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP) + `, wt.RepositoryID, wt.RepositoryPath); err != nil { + t.Fatalf("persist repository path: %v", err) + } + if _, err := store.db.ExecContext(ctx, `UPDATE task_sessions SET base_branch = ? WHERE id = ?`, wt.BaseBranch, wt.SessionID); err != nil { + t.Fatalf("persist session base branch: %v", err) + } + runGit(t, wt.Path, "commit", "--allow-empty", "-m", "archive before integration") + wantHead := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD")) + if _, err := store.db.ExecContext(ctx, `UPDATE tasks SET archived_at = CURRENT_TIMESTAMP WHERE id = ?`, wt.TaskID); err != nil { + t.Fatalf("archive task: %v", err) + } + archiveReceipt, err := mgr.CleanupWorktreesWithReceipt(ctx, []*Worktree{wt}) + if err != nil { + t.Fatalf("archive cleanup: %v", err) + } + if archiveReceipt.RetainedReasons[RetainedNotIntegrated] != 1 { + t.Fatalf("archive receipt = %+v, want not-integrated retention", archiveReceipt) + } + runGit(t, wt.RepositoryPath, "update-ref", "refs/heads/main", wantHead) + return mgr, store, wt, wantHead +} diff --git a/apps/backend/internal/worktree/errors.go b/apps/backend/internal/worktree/errors.go index 7c62ed44ec..b614b705da 100644 --- a/apps/backend/internal/worktree/errors.go +++ b/apps/backend/internal/worktree/errors.go @@ -96,8 +96,8 @@ var ( ErrInvalidRepoName = errors.New("repo name has no usable characters after sanitization") // ErrBranchUnrecoverable is returned by recreate when the worktree's - // branch no longer exists locally (archive deletes it via `git branch - // -D`) and could not be fetched from origin either. Callers can treat + // branch no longer exists locally, has no exact managed recovery head, + // and could not be fetched from origin either. Callers can treat // this as "prior work is gone" and fall back to a fresh worktree. ErrBranchUnrecoverable = errors.New("worktree branch no longer exists locally or on origin") diff --git a/apps/backend/internal/worktree/handoff_cleaner.go b/apps/backend/internal/worktree/handoff_cleaner.go index 9739d748bd..88e7a130a9 100644 --- a/apps/backend/internal/worktree/handoff_cleaner.go +++ b/apps/backend/internal/worktree/handoff_cleaner.go @@ -124,10 +124,13 @@ func (c *HandoffCleaner) CleanupPlainFolder(ctx context.Context, path string) er return nil } -// removeBranch is FALSE: handoffs cleanup releases the materialized -// workspace; the branch the agent created is left intact so any -// pushed PR / remote ref survives. Operators clean up branches via -// the existing branch-cleanup tooling. +// CleanupSingleRepoWorktree removes a single git worktree by ID via +// the existing Manager.RemoveByID, which already runs its own +// repository-scoped lock + git-worktree-remove + cleanup script. +// +// Terminal handoff cleanup uses the manager's metadata-aware compaction: only +// unambiguously managed, fully integrated local refs are removed; every other +// branch is left intact so pushed PR / remote refs and unpublished work survive. func (c *HandoffCleaner) CleanupSingleRepoWorktree(ctx context.Context, worktreeID string) error { if c.manager == nil { return errors.New("worktree manager not configured") @@ -136,7 +139,8 @@ func (c *HandoffCleaner) CleanupSingleRepoWorktree(ctx context.Context, worktree return errors.New("worktree id is required") } c.logger.Info("cleanup single-repo worktree", zap.String("worktree_id", worktreeID)) - return c.manager.RemoveByID(ctx, worktreeID, false) + _, err := c.manager.RemoveByIDWithReceipt(ctx, worktreeID) + return err } // CleanupMultiRepoRoot removes every per-repo worktree under a @@ -147,6 +151,9 @@ func (c *HandoffCleaner) CleanupMultiRepoRoot(ctx context.Context, rootPath stri if c.manager == nil { return errors.New("worktree manager not configured") } + if rootPath == "" { + return errors.New("multi-repo root path is required") + } if err := c.requireManagedRoot(rootPath); err != nil { return err } @@ -160,19 +167,32 @@ func (c *HandoffCleaner) CleanupMultiRepoRoot(ctx context.Context, rootPath stri if len(worktreeIDs) == 0 { return errors.New("multi-repo worktree inventory is empty") } - var removalErrors []error + receipt := newBranchCleanupReceipt() + var cleanupErrs []error + seen := make(map[string]struct{}, len(worktreeIDs)) for _, id := range worktreeIDs { if strings.TrimSpace(id) == "" { return errors.New("multi-repo worktree inventory contains an empty ID") } - if err := c.manager.RemoveByID(ctx, id, false); err != nil { + if _, ok := seen[id]; ok { + continue + } + seen[id] = struct{}{} + childReceipt, err := c.manager.RemoveByIDWithReceipt(ctx, id) + receipt.merge(childReceipt) + if err != nil { + cleanupErrs = append(cleanupErrs, fmt.Errorf("remove multi-repo worktree %s: %w", id, err)) c.logger.Warn("multi-repo worktree remove failed", zap.String("worktree_id", id), zap.Error(err)) - removalErrors = append(removalErrors, fmt.Errorf("remove worktree %s: %w", id, err)) + } + if childReceipt.RetainedReasons[RetainedActiveReference] > 0 { + cleanupErrs = append(cleanupErrs, + fmt.Errorf("multi-repo worktree %s retained by an active reference", id)) } } - if err := errors.Join(removalErrors...); err != nil { - return err + c.logger.Info("multi-repo worktree cleanup receipt", receipt.reasonFields()...) + if len(cleanupErrs) > 0 { + return errors.Join(cleanupErrs...) } if err := c.removeManagedDirectory(ctx, rootPath); err != nil { return fmt.Errorf("remove multi-repo root %s: %w", rootPath, err) diff --git a/apps/backend/internal/worktree/handoff_cleaner_test.go b/apps/backend/internal/worktree/handoff_cleaner_test.go index 2d9391a178..863cf1d032 100644 --- a/apps/backend/internal/worktree/handoff_cleaner_test.go +++ b/apps/backend/internal/worktree/handoff_cleaner_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/task/models" ) // newCleanerWithTasksRoot constructs a HandoffCleaner whose managed @@ -180,6 +181,47 @@ func TestCleanupRemoteEnvironmentRejectsUnimplementedDeletion(t *testing.T) { } } +func TestCleanupMultiRepoRoot_PreservesRootWhenChildIsShared(t *testing.T) { + mgr, store := newReferenceCleanupTestManager(t) + ctx := context.Background() + seedReferenceCleanupSession(t, store, "task-owner", "session-owner", models.TaskSessionStateRunning) + seedReferenceCleanupSession(t, store, "task-borrower", "session-borrower", models.TaskSessionStateRunning) + if err := store.linkSessionToEnvironment(ctx, "session-borrower", "env-owner"); err != nil { + t.Fatalf("link borrower session: %v", err) + } + + wt := createReferenceCleanupWorktree(t, mgr, "task-owner", "session-owner") + root := filepath.Dir(wt.Path) + cleaner := NewHandoffCleaner(mgr, newTestLogger()) + + if err := cleaner.CleanupMultiRepoRoot(ctx, root, []string{wt.ID}); err == nil { + t.Fatal("cleanup should remain retryable while a shared child is retained") + } + if _, err := os.Stat(wt.Path); err != nil { + t.Fatalf("shared child was removed: %v", err) + } + if _, err := os.Stat(root); err != nil { + t.Fatalf("multi-repo root was removed despite retained child: %v", err) + } +} + +func TestCleanupMultiRepoRoot_RemovesRootWhenOnlyBranchIsRetained(t *testing.T) { + mgr, store := newReferenceCleanupTestManager(t) + ctx := context.Background() + seedReferenceCleanupSession(t, store, "task-branch", "session-branch", models.TaskSessionStateCompleted) + wt := createReferenceCleanupWorktree(t, mgr, "task-branch", "session-branch") + runGit(t, wt.Path, "commit", "--allow-empty", "-m", "unintegrated branch") + root := filepath.Dir(wt.Path) + cleaner := NewHandoffCleaner(mgr, newTestLogger()) + + if err := cleaner.CleanupMultiRepoRoot(ctx, root, []string{wt.ID}); err != nil { + t.Fatalf("cleanup should remove root after removing child: %v", err) + } + if _, err := os.Stat(root); !os.IsNotExist(err) { + t.Fatalf("multi-repo root remains after child removal, stat error = %v", err) + } +} + func TestIsDescendant_HappyAndUnhappyPaths(t *testing.T) { cases := []struct { root, path string diff --git a/apps/backend/internal/worktree/manager.go b/apps/backend/internal/worktree/manager.go index 7643891952..1954febde9 100644 --- a/apps/backend/internal/worktree/manager.go +++ b/apps/backend/internal/worktree/manager.go @@ -27,6 +27,9 @@ const ( gitNoTags = "--no-tags" ) +// ArchivedBranchMaintenanceBatchLimit bounds one storage-maintenance revisit. +const ArchivedBranchMaintenanceBatchLimit = 100 + // repoLockEntry tracks a repository lock and its reference count. type repoLockEntry struct { mu *sync.Mutex @@ -120,6 +123,30 @@ type MultiRepoStore interface { GetWorktreeBySessionAndRepository(ctx context.Context, sessionID, repositoryID, branchSlug string) (*Worktree, error) } +// BranchMetadataStore is the fail-closed persistence capability required for +// managed-branch compaction. Stores without it retain every branch. +type BranchMetadataStore interface { + CountWorktreeBranchOwners(ctx context.Context, repositoryPath, branch string) (int, error) + PersistBranchRecoveryHead(ctx context.Context, worktreeID, expected, recoveryHead string) (bool, error) + PersistBranchCompactionComplete(ctx context.Context, worktreeID, expectedRecoveryHead string) (bool, error) + PersistBranchRecoveryRestored(ctx context.Context, worktreeID, expectedRecoveryHead string) (bool, error) +} + +// ArchivedBranchMaintenanceStore supplies only durable archived worktree +// candidates. The Manager remains responsible for every Git safety check and +// mutation after selection. +type ArchivedBranchMaintenanceStore interface { + ListArchivedBranchCandidates(ctx context.Context, limit int) ([]*Worktree, error) + IsArchivedBranchCandidate(ctx context.Context, worktreeID string) (bool, error) + PersistArchivedBranchRecoveryHead( + ctx context.Context, worktreeID, expected, recoveryHead string, + ) (bool, error) + PersistArchivedBranchCompactionComplete( + ctx context.Context, worktreeID, expectedRecoveryHead string, + ) (bool, error) + TouchArchivedBranchCandidate(ctx context.Context, worktreeID string) error +} + // NewManager creates a new worktree manager. func NewManager(cfg Config, store Store, log *logger.Logger) (*Manager, error) { if err := cfg.Validate(); err != nil { diff --git a/apps/backend/internal/worktree/manager_cleanup.go b/apps/backend/internal/worktree/manager_cleanup.go index 21d3e2d813..6fd78b453f 100644 --- a/apps/backend/internal/worktree/manager_cleanup.go +++ b/apps/backend/internal/worktree/manager_cleanup.go @@ -192,14 +192,39 @@ func (m *Manager) RemoveByID(ctx context.Context, worktreeID string, removeBranc if err != nil { return err } - m.enrichCleanupWorktreeFromCache(wt) - return m.removeWorktree(ctx, wt, removeBranch, WorktreeCleanupOptions{}) + wt = m.enrichCleanupWorktreeFromCache(ctx, wt) + receipt, err := m.removeWorktreeWithReceipt(ctx, wt, removeBranch, WorktreeCleanupOptions{}) + if !removeBranch { + m.logger.Info("managed branch cleanup receipt", receipt.reasonFields()...) + } + return err } -func (m *Manager) enrichCleanupWorktreeFromCache(wt *Worktree) { - if wt == nil || (wt.RepositoryPath != "" && wt.BaseBranch != "" && wt.CleanupHeadOID != "") { - return +// RemoveByIDWithReceipt removes one terminal worktree and safely compacts an +// eligible managed branch. It never force-deletes a branch. +func (m *Manager) RemoveByIDWithReceipt(ctx context.Context, worktreeID string) (BranchCleanupReceipt, error) { + wt, err := m.GetByID(ctx, worktreeID) + if err != nil { + return newBranchCleanupReceipt(), err } + wt = m.enrichCleanupWorktreeFromCache(ctx, wt) + receipt, err := m.removeWorktreeWithReceipt(ctx, wt, false, WorktreeCleanupOptions{}) + m.logger.Info("managed branch cleanup receipt", receipt.reasonFields()...) + return receipt, err +} + +func (m *Manager) enrichCleanupWorktreeFromCache(ctx context.Context, wt *Worktree) *Worktree { + if wt == nil { + return nil + } + clone := *wt + wt = &clone + m.enrichCleanupWorktreeFromCachedFields(wt, m.cachedCleanupWorktree(wt)) + m.enrichCleanupWorktreeFromStore(ctx, wt) + return wt +} + +func (m *Manager) cachedCleanupWorktree(wt *Worktree) *Worktree { m.mu.RLock() cached := m.worktrees[cacheKey(wt.SessionID, wt.RepositoryID, wt.BranchSlug)] if cached == nil { @@ -211,6 +236,10 @@ func (m *Manager) enrichCleanupWorktreeFromCache(wt *Worktree) { } } m.mu.RUnlock() + return cached +} + +func (m *Manager) enrichCleanupWorktreeFromCachedFields(wt, cached *Worktree) { if cached == nil || cached.ID != wt.ID { return } @@ -223,6 +252,36 @@ func (m *Manager) enrichCleanupWorktreeFromCache(wt *Worktree) { if wt.CleanupHeadOID == "" && !wt.CleanupHeadOIDUnavailable { wt.CleanupHeadOID = cached.CleanupHeadOID } + if wt.BranchOwner == "" { + wt.BranchOwner = cached.BranchOwner + } + if wt.IntegrationRef == "" { + wt.IntegrationRef = cached.IntegrationRef + } + if wt.RecoveryHeadSHA == "" { + wt.RecoveryHeadSHA = cached.RecoveryHeadSHA + } + if wt.BranchCompactedAt == nil { + wt.BranchCompactedAt = cached.BranchCompactedAt + } +} + +func (m *Manager) enrichCleanupWorktreeFromStore(ctx context.Context, wt *Worktree) { + if m.store == nil || wt.ID == "" { + return + } + current, err := m.store.GetWorktreeByID(ctx, wt.ID) + if err != nil || current == nil || current.ID != wt.ID { + return + } + // A durable cleanup snapshot can outlive the manager cache. Reload the + // internal branch state from the current row when it is available so a + // newer recovery or compaction transition always wins over stale snapshot + // data. + wt.BranchOwner = current.BranchOwner + wt.IntegrationRef = current.IntegrationRef + wt.RecoveryHeadSHA = current.RecoveryHeadSHA + wt.BranchCompactedAt = current.BranchCompactedAt } // CaptureCleanupHeadOIDs records the checkout commit for each worktree before @@ -235,7 +294,7 @@ func (m *Manager) CaptureCleanupHeadOIDs(ctx context.Context, worktrees []*Workt if wt == nil || wt.ID == "" { continue } - m.enrichCleanupWorktreeFromCache(wt) + wt = m.enrichCleanupWorktreeFromCache(ctx, wt) if strings.TrimSpace(wt.RepositoryPath) == "" || strings.TrimSpace(wt.Path) == "" { // Older environment rows can outlive their repository/session rows. // Keep them in the durable snapshot, but let the later cleanup audit @@ -289,13 +348,18 @@ func (m *Manager) CaptureCleanupHeadOIDs(ctx context.Context, worktrees []*Workt // removeWorktree performs the actual removal of a worktree. func (m *Manager) removeWorktree( - ctx context.Context, - wt *Worktree, - removeBranch bool, - options WorktreeCleanupOptions, + ctx context.Context, wt *Worktree, removeBranch bool, options WorktreeCleanupOptions, ) error { + _, err := m.removeWorktreeWithReceipt(ctx, wt, removeBranch, options) + return err +} + +func (m *Manager) removeWorktreeWithReceipt( + ctx context.Context, wt *Worktree, removeBranch bool, options WorktreeCleanupOptions, +) (BranchCleanupReceipt, error) { + receipt := newBranchCleanupReceipt() if wt == nil { - return errors.New("worktree cleanup requires a worktree") + return receipt, errors.New("worktree cleanup requires a worktree") } // Cleanup runs after the inventory snapshot has been selected. Keep a // private copy so a later session/path projection update cannot redirect @@ -303,17 +367,16 @@ func (m *Manager) removeWorktree( worktreeSnapshot := *wt wt = &worktreeSnapshot if strings.TrimSpace(wt.RepositoryPath) == "" || strings.TrimSpace(wt.Path) == "" { - return errors.New("worktree cleanup requires repository and worktree paths") + return receipt, errors.New("worktree cleanup requires repository and worktree paths") } // Serialize cleanup with create/recreate operations that target the same // path. The path lock is acquired before the repository lock to match the // creation order and avoid a lock-order deadlock. releasePath, err := acquireWorktreeTargetPath(ctx, wt.Path) if err != nil { - return fmt.Errorf("lock worktree cleanup path: %w", err) + return receipt, fmt.Errorf("lock worktree cleanup path: %w", err) } defer releasePath() - // Get repository lock repoLock := m.getRepoLock(wt.RepositoryPath) repoLock.Lock() @@ -328,23 +391,15 @@ func (m *Manager) removeWorktree( // and authorize deletion of a workspace another task still holds. activeReferences, err := m.CountActiveWorktreeReferences(ctx, wt.ID, nil) if err != nil { - return fmt.Errorf("count active references for worktree %s: %w", wt.ID, err) + return receipt, fmt.Errorf("count active references for worktree %s: %w", wt.ID, err) } - if activeReferences > 0 { - // The worktree is owned by a task environment that another task's - // session still references. The single environment-repository row is - // the only record, so there is no per-session reference to release — - // leave the row and directory untouched. - m.logger.Info("preserved worktree still referenced by another task session", - zap.String("worktree_id", wt.ID), - zap.String("session_id", wt.SessionID), - zap.Int("non_deleted_references", activeReferences)) - return nil + if retainedReceipt, retained := m.retainReferencedWorktree(wt, removeBranch, activeReferences); retained { + return retainedReceipt, nil } audit, err := m.auditWorktreeCleanup(ctx, wt, removeBranch, options) if err != nil { - return fmt.Errorf("audit worktree cleanup %s: %w", wt.ID, err) + return receipt, fmt.Errorf("audit worktree cleanup %s: %w", wt.ID, err) } defer func() { if audit.pathHandle != nil { @@ -358,39 +413,84 @@ func (m *Manager) removeWorktree( m.runWorktreeCleanupScript(ctx, wt) if err := m.completeAuditedWorktreeCleanup(ctx, wt, audit, removeBranch); err != nil { - return fmt.Errorf("complete worktree cleanup %s: %w", wt.ID, err) + return receipt, fmt.Errorf("complete worktree cleanup %s: %w", wt.ID, err) + } + if err := m.finalizeRemovedWorktree(ctx, wt); err != nil { + return receipt, err + } + if !removeBranch { + receipt = m.compactManagedBranch(ctx, wt) } - // Update store + m.logger.Info("removed worktree", + zap.String("task_id", wt.TaskID), + zap.String("worktree_id", wt.ID), + zap.String("path", wt.Path), + zap.Bool("branch_removed", removeBranch || receipt.Deleted > 0), + zap.Bool("branch_force_removed", removeBranch)) + + return receipt, nil +} + +func (m *Manager) finalizeRemovedWorktree(ctx context.Context, wt *Worktree) error { if m.store != nil && wt.SessionID != "" { if err := m.ReleaseWorktreeReference(ctx, wt); err != nil { return fmt.Errorf("release worktree reference %s: %w", wt.ID, err) } } - // Update cache: delete the (session, repo) entry. Removing a worktree only - // affects its own repo; siblings on other repos must remain cached. + // Removing a worktree affects only its repository cache entry; sibling + // repositories in the same session remain cached. m.mu.Lock() + defer m.mu.Unlock() if wt.SessionID != "" { delete(m.worktrees, cacheKey(wt.SessionID, wt.RepositoryID, wt.BranchSlug)) } - m.mu.Unlock() - - m.logger.Info("removed worktree", - zap.String("task_id", wt.TaskID), - zap.String("worktree_id", wt.ID), - zap.String("path", wt.Path), - zap.Bool("branch_removed", removeBranch)) - return nil } +func (m *Manager) retainReferencedWorktree( + wt *Worktree, removeBranch bool, activeReferences int, +) (BranchCleanupReceipt, bool) { + receipt := newBranchCleanupReceipt() + if activeReferences <= 0 { + return receipt, false + } + // The worktree is owned by a task environment that another task's session + // still references. Leave the single environment-repository row and + // directory untouched because there is no per-session reference to release. + m.logger.Info("preserved worktree still referenced by another task session", + zap.String("worktree_id", wt.ID), + zap.String("session_id", wt.SessionID), + zap.Int("non_deleted_references", activeReferences)) + if !removeBranch { + receipt.Attempted = 1 + branchCleanupMetrics.Add("attempted", 1) + receipt.retain(RetainedActiveReference) + } + return receipt, true +} + // ReleaseWorktreeReference marks one session's association deleted without // removing the shared directory or branch. func (m *Manager) ReleaseWorktreeReference(ctx context.Context, wt *Worktree) error { if wt == nil || wt.SessionID == "" { return fmt.Errorf("session ID is required to release worktree reference") } + if m.store != nil && wt.ID != "" { + current, err := m.store.GetWorktreeByID(ctx, wt.ID) + if err != nil && !errors.Is(err, ErrWorktreeNotFound) { + return err + } + if current != nil { + // The cleanup snapshot may be stale. Keep the latest branch policy + // and recovery state while changing only this association's status. + wt.BranchOwner = current.BranchOwner + wt.IntegrationRef = current.IntegrationRef + wt.RecoveryHeadSHA = current.RecoveryHeadSHA + wt.BranchCompactedAt = current.BranchCompactedAt + } + } now := time.Now().UTC() wt.Status = StatusDeleted wt.DeletedAt = &now @@ -542,35 +642,111 @@ func (m *Manager) CleanupWorktreesWithOptions( return m.cleanupWorktrees(ctx, worktrees, true, options) } -// CleanupWorktreesPreservingBranches removes provided worktrees while retaining -// their local branch refs for later archive recovery. +// CleanupWorktreesPreservingBranches removes provided worktrees and compacts +// only managed branches proven fully integrated. The historical name remains +// for caller compatibility; unpublished and ambiguous branches are retained. func (m *Manager) CleanupWorktreesPreservingBranches(ctx context.Context, worktrees []*Worktree) error { - return m.cleanupWorktrees(ctx, worktrees, false, WorktreeCleanupOptions{}) + _, err := m.CleanupWorktreesWithReceipt(ctx, worktrees) + return err } -func (m *Manager) cleanupWorktrees( - ctx context.Context, - worktrees []*Worktree, - removeBranch bool, - options WorktreeCleanupOptions, -) error { - if len(worktrees) == 0 { - return nil - } +func (m *Manager) CleanupWorktreesWithReceipt( + ctx context.Context, worktrees []*Worktree, +) (BranchCleanupReceipt, error) { + receipt, err := m.cleanupWorktreesWithReceipt(ctx, worktrees, false, WorktreeCleanupOptions{}) + m.logger.Info("managed branch cleanup receipt", receipt.reasonFields()...) + return receipt, err +} - var errs []error - for _, wt := range worktrees { +// MaintainArchivedBranches revisits a bounded set of managed branches retained +// by archive cleanup. Candidate selection is durable metadata only; the normal +// manager policy still owns repository locking, liveness, ancestry, recovery, +// and ref deletion. +func (m *Manager) MaintainArchivedBranches( + ctx context.Context, limit int, +) (BranchCleanupReceipt, error) { + receipt := newBranchCleanupReceipt() + if limit <= 0 { + return receipt, nil + } + if limit > ArchivedBranchMaintenanceBatchLimit { + limit = ArchivedBranchMaintenanceBatchLimit + } + maintenanceStore, ok := m.store.(ArchivedBranchMaintenanceStore) + if !ok { + return receipt, nil + } + candidates, err := maintenanceStore.ListArchivedBranchCandidates(ctx, limit) + if err != nil { + return receipt, err + } + for _, wt := range candidates { if wt == nil { continue } - // Task-environment inventory rows can describe a repository slot - // without a materialized physical worktree. Never pass such a row to - // filesystem cleanup, because its path can be the source checkout. - if strings.TrimSpace(wt.ID) == "" { + candidateReceipt, candidateErr := m.maintainArchivedBranchCandidate(ctx, maintenanceStore, wt) + receipt.merge(candidateReceipt) + if candidateErr != nil && err == nil { + err = candidateErr + } + if touchErr := maintenanceStore.TouchArchivedBranchCandidate(ctx, wt.ID); touchErr != nil && err == nil { + err = touchErr + } + } + m.logger.Info("archived managed branch maintenance receipt", receipt.reasonFields()...) + return receipt, err +} + +func (m *Manager) maintainArchivedBranchCandidate( + ctx context.Context, maintenanceStore ArchivedBranchMaintenanceStore, wt *Worktree, +) (BranchCleanupReceipt, error) { + repoLock := m.getRepoLock(wt.RepositoryPath) + repoLock.Lock() + defer func() { + repoLock.Unlock() + m.releaseRepoLock(wt.RepositoryPath) + }() + + eligible, err := maintenanceStore.IsArchivedBranchCandidate(ctx, wt.ID) + if err == nil && eligible { + return m.compactArchivedManagedBranch(ctx, wt), nil + } + candidateReceipt := newBranchCleanupReceipt() + candidateReceipt.Attempted = 1 + branchCleanupMetrics.Add("attempted", 1) + candidateReceipt.retain(RetainedArchiveStateChanged) + return candidateReceipt, err +} + +func (m *Manager) cleanupWorktrees( + ctx context.Context, worktrees []*Worktree, removeBranch bool, options WorktreeCleanupOptions, +) error { + _, err := m.cleanupWorktreesWithReceipt(ctx, worktrees, removeBranch, options) + return err +} + +func (m *Manager) cleanupWorktreesWithReceipt( + ctx context.Context, worktrees []*Worktree, removeBranch bool, options WorktreeCleanupOptions, +) (BranchCleanupReceipt, error) { + receipt := newBranchCleanupReceipt() + if len(worktrees) == 0 { + return receipt, nil + } + + var errs []error + seen := make(map[string]struct{}, len(worktrees)) + for _, wt := range worktrees { + if wt == nil || strings.TrimSpace(wt.ID) == "" { continue } - m.enrichCleanupWorktreeFromCache(wt) - if err := m.removeWorktree(ctx, wt, removeBranch, options); err != nil { + if _, ok := seen[wt.ID]; ok { + continue + } + seen[wt.ID] = struct{}{} + wt = m.enrichCleanupWorktreeFromCache(ctx, wt) + branchReceipt, err := m.removeWorktreeWithReceipt(ctx, wt, removeBranch, options) + receipt.merge(branchReceipt) + if err != nil { m.logger.Warn("failed to remove worktree during batch cleanup", zap.String("task_id", wt.TaskID), zap.String("worktree_id", wt.ID), @@ -578,8 +754,7 @@ func (m *Manager) cleanupWorktrees( errs = append(errs, fmt.Errorf("cleanup worktree %s: %w", wt.ID, err)) } } - - return errors.Join(errs...) + return receipt, errors.Join(errs...) } // OnTaskDeleted cleans up all worktrees for a task when it is deleted. diff --git a/apps/backend/internal/worktree/manager_cleanup_branch_policy_test.go b/apps/backend/internal/worktree/manager_cleanup_branch_policy_test.go index 187070741f..752a46f29a 100644 --- a/apps/backend/internal/worktree/manager_cleanup_branch_policy_test.go +++ b/apps/backend/internal/worktree/manager_cleanup_branch_policy_test.go @@ -2,13 +2,77 @@ package worktree import ( "context" + "errors" "os" + "os/exec" + "path/filepath" "strings" + "sync" "testing" "github.com/kandev/kandev/internal/task/models" ) +type archivedBranchMaintainer interface { + MaintainArchivedBranches(context.Context, int) (BranchCleanupReceipt, error) +} + +type unarchivingMaintenanceStore struct { + *SQLiteStore + taskID string + checks int +} + +type activatingSessionMaintenanceStore struct { + *SQLiteStore + sessionID string + checks int +} + +type activateAfterCandidateCheckStore struct { + *SQLiteStore + sessionID string + checks int +} + +func (s *activatingSessionMaintenanceStore) IsArchivedBranchCandidate( + ctx context.Context, worktreeID string, +) (bool, error) { + s.checks++ + if s.checks == 2 { + if _, err := s.db.ExecContext(ctx, `UPDATE task_sessions SET state = ? WHERE id = ?`, + models.TaskSessionStateRunning, s.sessionID); err != nil { + return false, err + } + } + return s.SQLiteStore.IsArchivedBranchCandidate(ctx, worktreeID) +} + +func (s *activateAfterCandidateCheckStore) IsArchivedBranchCandidate( + ctx context.Context, worktreeID string, +) (bool, error) { + s.checks++ + eligible, err := s.SQLiteStore.IsArchivedBranchCandidate(ctx, worktreeID) + if err != nil || !eligible || s.checks != 2 { + return eligible, err + } + _, err = s.db.ExecContext(ctx, `UPDATE task_sessions SET state = ? WHERE id = ?`, + models.TaskSessionStateRunning, s.sessionID) + return eligible, err +} + +func (s *unarchivingMaintenanceStore) IsArchivedBranchCandidate( + ctx context.Context, worktreeID string, +) (bool, error) { + s.checks++ + if s.checks == 2 { + if _, err := s.db.ExecContext(ctx, `UPDATE tasks SET archived_at = NULL WHERE id = ?`, s.taskID); err != nil { + return false, err + } + } + return s.SQLiteStore.IsArchivedBranchCandidate(ctx, worktreeID) +} + func TestCleanupWorktrees_PreservesBranchWhenRequested(t *testing.T) { mgr, store := newReferenceCleanupTestManager(t) seedReferenceCleanupSession(t, store, "task-archive", "session-archive", models.TaskSessionStateCompleted) @@ -29,6 +93,593 @@ func TestCleanupWorktrees_PreservesBranchWhenRequested(t *testing.T) { assertWorktreeReferenceStatus(t, store, wt.ID, StatusDeleted) } +func TestCleanupWorktreesPreservingBranches_RetainsLegacyUnknownOwner(t *testing.T) { + mgr, store := newReferenceCleanupTestManager(t) + seedReferenceCleanupSession(t, store, "task-legacy", "session-legacy", models.TaskSessionStateCompleted) + wt := createReferenceCleanupWorktree(t, mgr, "task-legacy", "session-legacy") + wt.BranchOwner = BranchOwnerUnknown + if err := store.UpdateWorktree(context.Background(), wt); err != nil { + t.Fatalf("mark legacy metadata: %v", err) + } + + receipt, err := mgr.CleanupWorktreesWithReceipt(context.Background(), []*Worktree{wt}) + if err != nil { + t.Fatalf("legacy cleanup: %v", err) + } + if receipt.Deleted != 0 || receipt.RetainedReasons[RetainedUnknownOwner] != 1 { + t.Fatalf("legacy receipt = %+v", receipt) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "branch", "--list", wt.Branch)); got == "" { + t.Fatal("legacy branch was deleted") + } +} + +func TestCleanupWorktreesPreservingBranches_RetainsExternalBranch(t *testing.T) { + mgr, store := newReferenceCleanupTestManager(t) + seedReferenceCleanupSession(t, store, "task-external", "session-external", models.TaskSessionStateCompleted) + wt := createReferenceCleanupWorktree(t, mgr, "task-external", "session-external") + wt.BranchOwner = BranchOwnerExternal + if err := store.UpdateWorktree(context.Background(), wt); err != nil { + t.Fatalf("mark external metadata: %v", err) + } + + receipt, err := mgr.CleanupWorktreesWithReceipt(context.Background(), []*Worktree{wt}) + if err != nil { + t.Fatalf("external cleanup: %v", err) + } + if receipt.RetainedReasons[RetainedExternalOwner] != 1 { + t.Fatalf("external receipt = %+v", receipt) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "branch", "--list", wt.Branch)); got == "" { + t.Fatal("external branch was deleted") + } +} + +func TestCleanupWorktreesPreservingBranches_RetainsCanonicalRepositoryDefault(t *testing.T) { + mgr, store := newReferenceCleanupTestManager(t) + mgr.SetRepositoryProvider(&fakeRepoProvider{repo: &Repository{ + ID: "repository", DefaultBranch: "release", + }}) + ctx := context.Background() + seedReferenceCleanupSession(t, store, "task-default-terminal", "session-default-terminal", models.TaskSessionStateCompleted) + wt := createReferenceCleanupWorktree(t, mgr, "task-default-terminal", "session-default-terminal") + runGit(t, wt.Path, "commit", "--allow-empty", "-m", "terminal default protection") + wantHead := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD")) + runGit(t, wt.Path, "branch", "-m", "release") + wt.Branch = "release" + if err := store.UpdateWorktree(ctx, wt); err != nil { + t.Fatalf("persist canonical default branch: %v", err) + } + // The primary worktree remains on main, so liveness alone cannot protect + // release. Its exact head is integrated, making it otherwise eligible. + runGit(t, wt.RepositoryPath, "update-ref", "refs/heads/main", wantHead) + + receipt, err := mgr.CleanupWorktreesWithReceipt(ctx, []*Worktree{wt}) + if err != nil { + t.Fatalf("terminal cleanup: %v", err) + } + if receipt.Deleted != 0 || receipt.RetainedReasons[RetainedProtectedRef] != 1 { + t.Fatalf("terminal default-protection receipt = %+v", receipt) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "rev-parse", "refs/heads/release")); got != wantHead { + t.Fatalf("canonical default branch head = %q, want %q", got, wantHead) + } +} + +func TestCleanupWorktreesPreservingBranches_RetainsWhenDefaultProtectionUnavailable(t *testing.T) { + tests := []struct { + name string + provider RepositoryProvider + }{ + {name: "nil provider"}, + {name: "empty default", provider: &fakeRepoProvider{repo: &Repository{ID: "repository"}}}, + {name: "provider error", provider: &fakeRepoProvider{err: errors.New("repository unavailable")}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mgr, store := newReferenceCleanupTestManager(t) + if tt.provider != nil { + mgr.SetRepositoryProvider(tt.provider) + } else { + mgr.SetRepositoryProvider(nil) + } + seedReferenceCleanupSession(t, store, "task-default-unavailable", "session-default-unavailable", models.TaskSessionStateCompleted) + wt := createReferenceCleanupWorktree(t, mgr, "task-default-unavailable", "session-default-unavailable") + runGit(t, wt.Path, "commit", "--allow-empty", "-m", "unintegrated archive") + ctx := context.Background() + if _, err := store.db.ExecContext(ctx, ` + INSERT INTO repositories (id, workspace_id, name, local_path, created_at, updated_at) + VALUES (?, 'workspace', 'repository', ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP) + `, wt.RepositoryID, wt.RepositoryPath); err != nil { + t.Fatalf("persist repository metadata: %v", err) + } + if _, err := store.db.ExecContext(ctx, `UPDATE tasks SET archived_at = CURRENT_TIMESTAMP WHERE id = ?`, wt.TaskID); err != nil { + t.Fatalf("archive task: %v", err) + } + if err := mgr.CleanupWorktreesPreservingBranches(ctx, []*Worktree{wt}); err != nil { + t.Fatalf("archive cleanup: %v", err) + } + receipt, err := mgr.MaintainArchivedBranches(ctx, 1) + if err != nil { + t.Fatalf("cleanup: %v", err) + } + if receipt.Deleted != 0 || receipt.RetainedReasons[RetainedProtectedRefUnavailable] != 1 { + t.Fatalf("receipt = %+v", receipt) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "branch", "--list", wt.Branch)); got == "" { + t.Fatal("branch was deleted without canonical default protection") + } + }) + } +} + +func TestProtectedBranchNameRecognizesEquivalentRefForms(t *testing.T) { + for _, protectedRef := range []string{"main", "origin/main", "refs/heads/main", "refs/remotes/origin/main"} { + if !protectedBranchName("main", protectedRef) { + t.Errorf("main was not protected by %q", protectedRef) + } + } + if protectedBranchName("feature/task", "origin/main") { + t.Fatal("feature branch was treated as the protected main branch") + } +} + +func TestCleanupWorktreesPreservingBranches_RetainsAmbiguousOwner(t *testing.T) { + mgr, store := newReferenceCleanupTestManager(t) + ctx := context.Background() + seedReferenceCleanupSession(t, store, "task-ambiguous", "session-ambiguous", models.TaskSessionStateCompleted) + seedReferenceCleanupSession(t, store, "task-ambiguous-peer", "session-ambiguous-peer", models.TaskSessionStateCompleted) + wt := createReferenceCleanupWorktree(t, mgr, "task-ambiguous", "session-ambiguous") + if _, err := store.db.ExecContext(ctx, ` + INSERT INTO task_environment_repos ( + id, task_environment_id, repository_id, branch_slug, worktree_id, + worktree_path, worktree_branch, worktree_branch_owner, + worktree_integration_ref, position, status, created_at, updated_at + ) VALUES ( + 'ambiguous-peer', 'env-ambiguous-peer', 'repository', 'peer', 'ambiguous-peer', + '/missing', ?, 'kandev', 'main', 0, 'deleted', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP + ) + `, wt.Branch); err != nil { + t.Fatalf("insert ambiguous owner: %v", err) + } + + receipt, err := mgr.CleanupWorktreesWithReceipt(ctx, []*Worktree{wt}) + if err != nil { + t.Fatalf("ambiguous cleanup: %v", err) + } + if receipt.RetainedReasons[RetainedAmbiguousOwner] != 1 { + t.Fatalf("ambiguous receipt = %+v", receipt) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "branch", "--list", wt.Branch)); got == "" { + t.Fatal("ambiguous branch was deleted") + } +} + +func TestCleanupWorktreesPreservingBranches_DeletesOnlyLocalRef(t *testing.T) { + mgr, store := newReferenceCleanupTestManager(t) + seedReferenceCleanupSession(t, store, "task-remote", "session-remote", models.TaskSessionStateCompleted) + wt := createReferenceCleanupWorktree(t, mgr, "task-remote", "session-remote") + head := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD")) + remoteRef := "refs/remotes/origin/" + wt.Branch + runGit(t, wt.RepositoryPath, "update-ref", remoteRef, head) + + if err := mgr.CleanupWorktreesPreservingBranches(context.Background(), []*Worktree{wt}); err != nil { + t.Fatalf("remote ref cleanup: %v", err) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "rev-parse", "--verify", remoteRef)); got != head { + t.Fatalf("remote ref head = %q, want %q", got, head) + } +} + +func TestCleanupWorktreesPreservingBranches_DeletesExactExpectedLocalRef(t *testing.T) { + mgr, store := newReferenceCleanupTestManager(t) + seedReferenceCleanupSession(t, store, "task-exact-ref", "session-exact-ref", models.TaskSessionStateCompleted) + wt := createReferenceCleanupWorktree(t, mgr, "task-exact-ref", "session-exact-ref") + wantHead := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD")) + + realGit, err := exec.LookPath("git") + if err != nil { + t.Fatalf("find git: %v", err) + } + binDir := t.TempDir() + logPath := filepath.Join(binDir, "git-commands.log") + wrapper := filepath.Join(binDir, "git") + script := "#!/bin/sh\nprintf '%s\\n' \"$*\" >> \"" + logPath + "\"\nexec \"" + realGit + "\" \"$@\"\n" + if err := os.WriteFile(wrapper, []byte(script), 0o755); err != nil { + t.Fatalf("write git wrapper: %v", err) + } + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + if err := mgr.CleanupWorktreesPreservingBranches(context.Background(), []*Worktree{wt}); err != nil { + t.Fatalf("exact-ref cleanup: %v", err) + } + commands, err := os.ReadFile(logPath) + if err != nil { + t.Fatalf("read git command log: %v", err) + } + wantCommand := "update-ref -d refs/heads/" + wt.Branch + " " + wantHead + if !strings.Contains(string(commands), wantCommand) { + t.Fatalf("git commands = %q, want atomic exact-ref deletion %q", commands, wantCommand) + } + if strings.Contains(string(commands), "branch -d") || strings.Contains(string(commands), "branch -D") { + t.Fatalf("git commands used branch deletion instead of expected-SHA deletion: %q", commands) + } +} + +func TestCleanupWorktreesPreservingBranches_HeadRaceRemainsRetryable(t *testing.T) { + mgr, store := newReferenceCleanupTestManager(t) + ctx := context.Background() + seedReferenceCleanupSession(t, store, "task-head-race", "session-head-race", models.TaskSessionStateCompleted) + wt := createReferenceCleanupWorktree(t, mgr, "task-head-race", "session-head-race") + wantHead := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD")) + tree := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", wantHead+"^{tree}")) + racingHead := strings.TrimSpace(runGit(t, wt.Path, "commit-tree", tree, "-p", wantHead, "-m", "racing head")) + + realGit, err := exec.LookPath("git") + if err != nil { + t.Fatalf("find git: %v", err) + } + binDir := t.TempDir() + wrapper := filepath.Join(binDir, "git") + branchRef := "refs/heads/" + wt.Branch + script := "#!/bin/sh\n" + + "if [ \"$1\" = \"-c\" ] && [ \"$3\" = \"update-ref\" ] && [ \"$4\" = \"-d\" ]; then\n" + + " \"" + realGit + "\" -C \"$PWD\" update-ref \"" + branchRef + "\" \"" + racingHead + "\"\n" + + "fi\n" + + "exec \"" + realGit + "\" \"$@\"\n" + if err := os.WriteFile(wrapper, []byte(script), 0o755); err != nil { + t.Fatalf("write git wrapper: %v", err) + } + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + receipt, err := mgr.CleanupWorktreesWithReceipt(ctx, []*Worktree{wt}) + if err != nil { + t.Fatalf("head-race cleanup: %v", err) + } + if receipt.Deleted != 0 || receipt.RetainedReasons[RetainedHeadChanged] != 1 { + t.Fatalf("head-race receipt = %+v, want head-changed retention", receipt) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "rev-parse", branchRef)); got != racingHead { + t.Fatalf("racing branch head = %q, want %q", got, racingHead) + } + archived, err := store.GetWorktreeByID(ctx, wt.ID) + if err != nil { + t.Fatalf("load raced worktree: %v", err) + } + if archived.RecoveryHeadSHA != "" { + t.Fatalf("raced recovery head = %q, want empty so maintenance can retry", archived.RecoveryHeadSHA) + } +} + +func TestCleanupWorktreesPreservingBranches_ConcurrentCleanupDeletesOnce(t *testing.T) { + mgr, store := newReferenceCleanupTestManager(t) + seedReferenceCleanupSession(t, store, "task-race", "session-race", models.TaskSessionStateCompleted) + wt := createReferenceCleanupWorktree(t, mgr, "task-race", "session-race") + + var wg sync.WaitGroup + receipts := make(chan BranchCleanupReceipt, 2) + errs := make(chan error, 2) + for range 2 { + wg.Add(1) + go func() { + defer wg.Done() + receipt, err := mgr.CleanupWorktreesWithReceipt(context.Background(), []*Worktree{wt}) + receipts <- receipt + errs <- err + }() + } + wg.Wait() + close(receipts) + close(errs) + for err := range errs { + if err != nil { + t.Fatalf("concurrent cleanup: %v", err) + } + } + deleted := 0 + for receipt := range receipts { + deleted += receipt.Deleted + } + if deleted != 1 { + t.Fatalf("concurrent deleted count = %d, want 1", deleted) + } +} + +func TestCleanupWorktreesWithReceipt_DeduplicatesWorktreeIDs(t *testing.T) { + mgr, store := newReferenceCleanupTestManager(t) + seedReferenceCleanupSession(t, store, "task-dedup", "session-dedup", models.TaskSessionStateCompleted) + wt := createReferenceCleanupWorktree(t, mgr, "task-dedup", "session-dedup") + + receipt, err := mgr.CleanupWorktreesWithReceipt(context.Background(), []*Worktree{wt, wt}) + if err != nil { + t.Fatalf("deduplicated cleanup: %v", err) + } + if receipt.Attempted != 1 || receipt.Deleted != 1 { + t.Fatalf("deduplicated receipt = %+v, want one attempted deletion", receipt) + } +} + +func TestCleanupWorktreesPreservingBranches_RemovesFullyMergedManagedBranch(t *testing.T) { + mgr, store := newReferenceCleanupTestManager(t) + seedReferenceCleanupSession(t, store, "task-merged", "session-merged", models.TaskSessionStateCompleted) + wt := createReferenceCleanupWorktree(t, mgr, "task-merged", "session-merged") + + if err := mgr.CleanupWorktreesPreservingBranches(context.Background(), []*Worktree{wt}); err != nil { + t.Fatalf("merged branch 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, "branch", "--list", wt.Branch)); got != "" { + t.Fatalf("fully merged managed branch remains after cleanup: %q", got) + } + assertWorktreeReferenceStatus(t, store, wt.ID, StatusDeleted) +} + +func TestMaintainArchivedBranches_CompactsAfterIntegrationAndRestoresExactHead(t *testing.T) { + mgr, store := newReferenceCleanupTestManager(t) + ctx := context.Background() + seedReferenceCleanupSession(t, store, "task-maintenance", "session-maintenance", models.TaskSessionStateCompleted) + wt := createReferenceCleanupWorktree(t, mgr, "task-maintenance", "session-maintenance") + if _, err := store.db.ExecContext(ctx, ` + INSERT INTO repositories (id, workspace_id, name, local_path, created_at, updated_at) + VALUES (?, 'workspace', 'repository', ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP) + `, wt.RepositoryID, wt.RepositoryPath); err != nil { + t.Fatalf("persist repository path: %v", err) + } + if _, err := store.db.ExecContext(ctx, ` + UPDATE task_sessions SET base_branch = ? WHERE id = ? + `, wt.BaseBranch, wt.SessionID); err != nil { + t.Fatalf("persist session base branch: %v", err) + } + runGit(t, wt.Path, "commit", "--allow-empty", "-m", "archive before integration") + wantHead := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD")) + + if _, err := store.db.ExecContext(ctx, ` + UPDATE tasks SET archived_at = CURRENT_TIMESTAMP WHERE id = ? + `, wt.TaskID); err != nil { + t.Fatalf("archive task: %v", err) + } + archiveReceipt, err := mgr.CleanupWorktreesWithReceipt(ctx, []*Worktree{wt}) + if err != nil { + t.Fatalf("archive cleanup: %v", err) + } + if archiveReceipt.RetainedReasons[RetainedNotIntegrated] != 1 { + t.Fatalf("archive receipt = %+v, want not-integrated retention", archiveReceipt) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "rev-parse", wt.Branch)); got != wantHead { + t.Fatalf("archive retained branch head = %q, want %q", got, wantHead) + } + + // Simulate the later local integration that the archive path could not + // observe. Storage maintenance must revisit the durable archived row rather + // than scan branch names. + runGit(t, wt.RepositoryPath, "update-ref", "refs/heads/main", wantHead) + maintainer, ok := any(mgr).(archivedBranchMaintainer) + if !ok { + t.Fatal("worktree manager does not implement bounded archived-branch maintenance") + } + maintenanceReceipt, err := maintainer.MaintainArchivedBranches(ctx, 1) + if err != nil { + t.Fatalf("archived branch maintenance: %v", err) + } + if maintenanceReceipt.Attempted != 1 || maintenanceReceipt.Deleted != 1 { + t.Fatalf("maintenance receipt = %+v, want one attempted deletion", maintenanceReceipt) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "branch", "--list", wt.Branch)); got != "" { + t.Fatalf("integrated archived branch remains after maintenance: %q", got) + } + + if _, err := store.db.ExecContext(ctx, ` + UPDATE tasks SET archived_at = NULL WHERE id = ? + `, wt.TaskID); err != nil { + t.Fatalf("unarchive task: %v", err) + } + restored, err := mgr.Create(ctx, CreateRequest{ + TaskID: wt.TaskID, + SessionID: wt.SessionID, + WorktreeID: wt.ID, + TaskTitle: "Archived maintenance", + RepositoryID: wt.RepositoryID, + RepositoryPath: wt.RepositoryPath, + BaseBranch: wt.BaseBranch, + IntegrationRef: wt.IntegrationRef, + TaskDirName: wt.TaskDirName, + RepoName: "repository", + }) + if err != nil { + t.Fatalf("restore after maintenance: %v", err) + } + if got := strings.TrimSpace(runGit(t, restored.Path, "rev-parse", "HEAD")); got != wantHead { + t.Fatalf("restored HEAD = %q, want exact archived head %q", got, wantHead) + } + persistedRestored, err := store.GetWorktreeByID(ctx, restored.ID) + if err != nil { + t.Fatalf("load restored worktree: %v", err) + } + if persistedRestored.BranchCompactedAt != nil { + t.Fatalf("restored worktree kept stale compaction marker: %v", persistedRestored.BranchCompactedAt) + } +} + +func TestMaintainArchivedBranches_UnarchiveAfterSelectionRetainsBranch(t *testing.T) { + mgr, store := newReferenceCleanupTestManager(t) + ctx := context.Background() + seedReferenceCleanupSession(t, store, "task-unarchive-race", "session-unarchive-race", models.TaskSessionStateCompleted) + wt := createReferenceCleanupWorktree(t, mgr, "task-unarchive-race", "session-unarchive-race") + if _, err := store.db.ExecContext(ctx, ` + INSERT INTO repositories (id, workspace_id, name, local_path, created_at, updated_at) + VALUES (?, 'workspace', 'repository', ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP) + `, wt.RepositoryID, wt.RepositoryPath); err != nil { + t.Fatalf("persist repository path: %v", err) + } + if _, err := store.db.ExecContext(ctx, `UPDATE task_sessions SET base_branch = ? WHERE id = ?`, wt.BaseBranch, wt.SessionID); err != nil { + t.Fatalf("persist session base branch: %v", err) + } + runGit(t, wt.Path, "commit", "--allow-empty", "-m", "archive before unarchive race") + wantHead := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD")) + if _, err := store.db.ExecContext(ctx, `UPDATE tasks SET archived_at = CURRENT_TIMESTAMP WHERE id = ?`, wt.TaskID); err != nil { + t.Fatalf("archive task: %v", err) + } + if _, err := mgr.CleanupWorktreesWithReceipt(ctx, []*Worktree{wt}); err != nil { + t.Fatalf("archive cleanup: %v", err) + } + runGit(t, wt.RepositoryPath, "update-ref", "refs/heads/main", wantHead) + + mgr.store = &unarchivingMaintenanceStore{SQLiteStore: store, taskID: wt.TaskID} + receipt, err := mgr.MaintainArchivedBranches(ctx, 1) + if err != nil { + t.Fatalf("maintenance during unarchive race: %v", err) + } + if receipt.Deleted != 0 || receipt.RetainedReasons[RetainedArchiveStateChanged] != 1 { + t.Fatalf("unarchive-race receipt = %+v, want archive-state retention", receipt) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "rev-parse", wt.Branch)); got != wantHead { + t.Fatalf("unarchive race changed branch head = %q, want %q", got, wantHead) + } +} + +func TestMaintainArchivedBranches_ActiveSessionAfterSelectionRetainsBranch(t *testing.T) { + mgr, store, wt, wantHead := archivedIntegratedBranchForMaintenance(t, "session-active-race") + mgr.store = &activatingSessionMaintenanceStore{ + SQLiteStore: store, + sessionID: wt.SessionID, + } + + receipt, err := mgr.MaintainArchivedBranches(context.Background(), 1) + if err != nil { + t.Fatalf("maintenance during session activation: %v", err) + } + if receipt.Deleted != 0 || receipt.RetainedReasons[RetainedArchiveStateChanged] != 1 { + t.Fatalf("maintenance receipt = %+v, want archive-state retention", receipt) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "rev-parse", "refs/heads/"+wt.Branch)); got != wantHead { + t.Fatalf("branch head after session activation = %q, want %q", got, wantHead) + } +} + +func TestMaintainArchivedBranches_ActiveSessionBeforeDeleteRetainsBranch(t *testing.T) { + mgr, store, wt, wantHead := archivedIntegratedBranchForMaintenance(t, "session-delete-race") + branchRef := "refs/heads/" + wt.Branch + mgr.store = &activateAfterCandidateCheckStore{ + SQLiteStore: store, + sessionID: wt.SessionID, + } + + receipt, err := mgr.MaintainArchivedBranches(context.Background(), 1) + if err != nil { + t.Fatalf("maintenance during delete race: %v", err) + } + if receipt.Deleted != 0 || receipt.RetainedReasons[RetainedArchiveStateChanged] != 1 { + t.Fatalf("maintenance receipt = %+v, want archive-state retention", receipt) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "rev-parse", branchRef)); got != wantHead { + t.Fatalf("branch head after session activation before delete = %q, want %q", got, wantHead) + } +} + +func TestMaintainArchivedBranches_DoesNotSelectActiveSession(t *testing.T) { + mgr, store, wt, wantHead := archivedIntegratedBranchForMaintenance(t, "session-already-active") + if _, err := store.db.ExecContext(context.Background(), `UPDATE task_sessions SET state = ? WHERE id = ?`, + models.TaskSessionStateRunning, wt.SessionID); err != nil { + t.Fatalf("activate archived task session: %v", err) + } + + receipt, err := mgr.MaintainArchivedBranches(context.Background(), 1) + if err != nil { + t.Fatalf("maintenance with active session: %v", err) + } + if receipt.Attempted != 0 || receipt.Deleted != 0 { + t.Fatalf("maintenance receipt = %+v, want active session excluded from selection", receipt) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "rev-parse", "refs/heads/"+wt.Branch)); got != wantHead { + t.Fatalf("branch head with active session = %q, want %q", got, wantHead) + } +} + +func TestMaintainArchivedBranches_ProtectsCanonicalRepositoryDefault(t *testing.T) { + mgr, store := newReferenceCleanupTestManager(t) + mgr.SetRepositoryProvider(&fakeRepoProvider{repo: &Repository{ + ID: "repository", + DefaultBranch: "release", + }}) + ctx := context.Background() + seedReferenceCleanupSession(t, store, "task-default-protection", "session-default-protection", models.TaskSessionStateCompleted) + wt := createReferenceCleanupWorktree(t, mgr, "task-default-protection", "session-default-protection") + oldBranch := wt.Branch + if _, err := store.db.ExecContext(ctx, ` + INSERT INTO repositories (id, workspace_id, name, local_path, default_branch, created_at, updated_at) + VALUES (?, 'workspace', 'repository', ?, 'release', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP) + `, wt.RepositoryID, wt.RepositoryPath); err != nil { + t.Fatalf("persist repository metadata: %v", err) + } + runGit(t, wt.Path, "commit", "--allow-empty", "-m", "default protection") + wantHead := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD")) + if _, err := store.db.ExecContext(ctx, `UPDATE tasks SET archived_at = CURRENT_TIMESTAMP WHERE id = ?`, wt.TaskID); err != nil { + t.Fatalf("archive task: %v", err) + } + if _, err := mgr.CleanupWorktreesWithReceipt(ctx, []*Worktree{wt}); err != nil { + t.Fatalf("archive cleanup: %v", err) + } + // Model a locally integrated archived branch whose canonical name is the + // persisted repository default, but which is not checked out anywhere. + runGit(t, wt.RepositoryPath, "branch", "-m", oldBranch, "release") + if _, err := store.db.ExecContext(ctx, ` + UPDATE task_environment_repos + SET worktree_branch = 'release', worktree_integration_ref = 'main' + WHERE worktree_id = ?`, wt.ID); err != nil { + t.Fatalf("rename archived candidate: %v", err) + } + runGit(t, wt.RepositoryPath, "update-ref", "refs/heads/main", wantHead) + if _, err := store.db.ExecContext(ctx, `UPDATE task_sessions SET base_branch = '' WHERE id = ?`, wt.SessionID); err != nil { + t.Fatalf("clear archived base branch: %v", err) + } + + receipt, err := mgr.MaintainArchivedBranches(ctx, 1) + if err != nil { + t.Fatalf("archived maintenance: %v", err) + } + if receipt.Deleted != 0 || receipt.RetainedReasons[RetainedProtectedRef] != 1 { + t.Fatalf("default-protection receipt = %+v", receipt) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "rev-parse", "release")); got != wantHead { + t.Fatalf("canonical default branch head = %q, want %q", got, wantHead) + } +} + +func TestRemoveByID_RemovesFullyMergedManagedBranch(t *testing.T) { + store := newMockStore() + mgr, err := NewManager(newTestConfig(t), store, newTestLogger()) + if err != nil { + t.Fatalf("NewManager: %v", err) + } + mgr.SetRepositoryProvider(&fakeRepoProvider{repo: &Repository{ + ID: "repository", DefaultBranch: "main", + }}) + wt, err := mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-direct", + SessionID: "session-direct", + TaskTitle: "Direct cleanup", + RepositoryID: "repository", + RepositoryPath: initGitRepoWithRemote(t), + BaseBranch: "main", + IntegrationRef: "main", + TaskDirName: "task-direct", + RepoName: "repository", + }) + if err != nil { + t.Fatalf("create direct worktree: %v", err) + } + + if err := mgr.RemoveByID(context.Background(), wt.ID, false); err != nil { + t.Fatalf("RemoveByID: %v", err) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "branch", "--list", wt.Branch)); got != "" { + t.Fatalf("fully merged direct branch remains: %q", got) + } +} + func TestCleanupWorktrees_RemovesBranchByDefault(t *testing.T) { mgr, store := newReferenceCleanupTestManager(t) seedReferenceCleanupSession(t, store, "task-delete", "session-delete", models.TaskSessionStateCompleted) diff --git a/apps/backend/internal/worktree/manager_cleanup_dirty.go b/apps/backend/internal/worktree/manager_cleanup_dirty.go index eebb9fb35e..8b2c69948c 100644 --- a/apps/backend/internal/worktree/manager_cleanup_dirty.go +++ b/apps/backend/internal/worktree/manager_cleanup_dirty.go @@ -37,7 +37,7 @@ func (m *Manager) InspectDirtyWorktrees( if wt == nil || strings.TrimSpace(wt.ID) == "" { continue } - m.enrichCleanupWorktreeFromCache(wt) + m.enrichCleanupWorktreeFromCache(ctx, wt) if strings.TrimSpace(wt.RepositoryPath) == "" || strings.TrimSpace(wt.Path) == "" { return nil, fmt.Errorf("worktree %s is missing repository or worktree path metadata", wt.ID) } diff --git a/apps/backend/internal/worktree/manager_cleanup_recovery_test.go b/apps/backend/internal/worktree/manager_cleanup_recovery_test.go index cec35d2abf..dbfc235fdc 100644 --- a/apps/backend/internal/worktree/manager_cleanup_recovery_test.go +++ b/apps/backend/internal/worktree/manager_cleanup_recovery_test.go @@ -7,6 +7,7 @@ import ( "path/filepath" "strings" "testing" + "time" "github.com/kandev/kandev/internal/task/models" ) @@ -15,6 +16,41 @@ type cleanupOrderRecordingScriptHandler struct { cleanupRuns int } +func TestReleaseWorktreeReference_PreservesCurrentBranchRecoveryMetadata(t *testing.T) { + mgr, store := newReferenceCleanupTestManager(t) + ctx := context.Background() + seedReferenceCleanupSession(t, store, "task-release-metadata", "session-release-metadata", models.TaskSessionStateCompleted) + current := createReferenceCleanupWorktree(t, mgr, "task-release-metadata", "session-release-metadata") + compactedAt := time.Now().UTC() + current.RecoveryHeadSHA = "current-recovery-head" + current.BranchCompactedAt = &compactedAt + if err := store.UpdateWorktree(ctx, current); err != nil { + t.Fatalf("persist current recovery metadata: %v", err) + } + + stale := *current + stale.BranchOwner = "" + stale.IntegrationRef = "" + stale.RecoveryHeadSHA = "stale-recovery-head" + stale.BranchCompactedAt = nil + if err := mgr.ReleaseWorktreeReference(ctx, &stale); err != nil { + t.Fatalf("release stale worktree reference: %v", err) + } + + persisted, err := store.GetWorktreeByID(ctx, current.ID) + if err != nil { + t.Fatalf("load released worktree: %v", err) + } + if persisted.BranchOwner != BranchOwnerManaged || persisted.IntegrationRef != "main" { + t.Fatalf("released branch metadata = owner %q integration %q, want managed/main", + persisted.BranchOwner, persisted.IntegrationRef) + } + if persisted.RecoveryHeadSHA != "current-recovery-head" || persisted.BranchCompactedAt == nil { + t.Fatalf("released recovery metadata = head %q compacted %v, want current state", + persisted.RecoveryHeadSHA, persisted.BranchCompactedAt) + } +} + func (h *cleanupOrderRecordingScriptHandler) ExecuteSetupScript(context.Context, ScriptExecutionRequest) error { return nil } @@ -40,6 +76,18 @@ func (s *failingReleaseStore) UpdateWorktree(ctx context.Context, wt *Worktree) return s.SQLiteStore.UpdateWorktree(ctx, wt) } +func TestCleanupWorktreesPreservingBranches_RetainsBranchWhenReleaseFails(t *testing.T) { + mgr, store := newReferenceCleanupTestManager(t) + seedReferenceCleanupSession(t, store, "task-preserve-retry", "session-preserve-retry", models.TaskSessionStateCompleted) + wt := createReferenceCleanupWorktree(t, mgr, "task-preserve-retry", "session-preserve-retry") + + mgr.store = &failingReleaseStore{SQLiteStore: store, failUpdate: true} + if err := mgr.CleanupWorktreesPreservingBranches(context.Background(), []*Worktree{wt}); err == nil { + t.Fatal("cleanup succeeded despite injected reference-release failure") + } + assertCleanupBranchPresent(t, wt.RepositoryPath, wt.Branch) +} + type swappingCleanupScriptHandler struct{} func (h *swappingCleanupScriptHandler) ExecuteSetupScript(context.Context, ScriptExecutionRequest) error { @@ -117,7 +165,9 @@ func TestCleanupWorktreesPreservingBranches_RetriesAfterReleaseFailure(t *testin if err := mgr.CleanupWorktreesPreservingBranches(context.Background(), []*Worktree{wt}); err != nil { t.Fatalf("retry pathless branch-preserving cleanup: %v", err) } - assertCleanupBranchPresent(t, wt.RepositoryPath, wt.Branch) + // The retry has now released the reference successfully, so the eligible + // fully integrated managed branch is safely compacted. + assertCleanupBranchAbsent(t, wt.RepositoryPath, wt.Branch) assertWorktreeReferenceStatus(t, store, wt.ID, StatusDeleted) } diff --git a/apps/backend/internal/worktree/manager_cleanup_shared_reference_test.go b/apps/backend/internal/worktree/manager_cleanup_shared_reference_test.go index 6d8829ef86..62853ee7ad 100644 --- a/apps/backend/internal/worktree/manager_cleanup_shared_reference_test.go +++ b/apps/backend/internal/worktree/manager_cleanup_shared_reference_test.go @@ -33,8 +33,12 @@ func TestCleanupWorktrees_PreservesSharedActiveReference(t *testing.T) { t.Fatalf("active foreign worktree references = %d, want 1", count) } - if err := mgr.CleanupWorktrees(ctx, []*Worktree{wt}); err != nil { - t.Fatalf("CleanupWorktrees: %v", err) + receipt, err := mgr.CleanupWorktreesWithReceipt(ctx, []*Worktree{wt}) + if err != nil { + t.Fatalf("CleanupWorktreesWithReceipt: %v", err) + } + if receipt.Attempted != 1 || receipt.RetainedReasons[RetainedActiveReference] != 1 { + t.Fatalf("shared-reference receipt = %+v", receipt) } if _, err := os.Stat(wt.Path); err != nil { @@ -149,6 +153,9 @@ func newReferenceCleanupTestManager(t *testing.T) (*Manager, *SQLiteStore) { if err != nil { t.Fatalf("NewManager: %v", err) } + mgr.SetRepositoryProvider(&fakeRepoProvider{repo: &Repository{ + ID: "repository", DefaultBranch: "main", + }}) return mgr, store } @@ -199,6 +206,7 @@ func createReferenceCleanupWorktree(t *testing.T, mgr *Manager, taskID, sessionI RepositoryID: "repository", RepositoryPath: initGitRepoWithRemote(t), BaseBranch: "main", + IntegrationRef: "main", TaskDirName: taskID, RepoName: "repository", }) diff --git a/apps/backend/internal/worktree/manager_git.go b/apps/backend/internal/worktree/manager_git.go index 51e5de2b46..923f2d3678 100644 --- a/apps/backend/internal/worktree/manager_git.go +++ b/apps/backend/internal/worktree/manager_git.go @@ -286,6 +286,84 @@ func (m *Manager) BranchRecoveryStatus(ctx context.Context, repoPath, branch str return BranchStatusMissing } +// RecoverBranchStatus restores a safely compacted managed branch from its +// persisted exact head before reporting status. It performs no fetch. +func (m *Manager) RecoverBranchStatus(ctx context.Context, wt *Worktree) string { + if wt == nil || wt.RepositoryPath == "" || wt.Branch == "" { + return BranchStatusMissing + } + repoLock := m.getRepoLock(wt.RepositoryPath) + repoLock.Lock() + defer func() { + repoLock.Unlock() + m.releaseRepoLock(wt.RepositoryPath) + }() + branchRef := "refs/heads/" + wt.Branch + if exists, err := m.branchExists(ctx, wt.RepositoryPath, branchRef); err == nil && exists { + return m.existingRecoveredBranchStatus(ctx, wt, branchRef) + } + if m.restoreManagedBranchFromRecoveryHeadLocked(ctx, wt) == nil && wt.RecoveryHeadSHA != "" { + return BranchStatusLocal + } + return m.BranchRecoveryStatus(ctx, wt.RepositoryPath, wt.Branch) +} + +func (m *Manager) existingRecoveredBranchStatus(ctx context.Context, wt *Worktree, branchRef string) string { + if wt.RecoveryHeadSHA == "" { + return BranchStatusLocal + } + current, err := m.resolveCommit(ctx, wt.RepositoryPath, branchRef) + if err != nil || !strings.EqualFold(current, wt.RecoveryHeadSHA) { + return BranchStatusMissing + } + if m.finalizeRestoredManagedBranch(ctx, wt, current) != nil { + return BranchStatusMissing + } + return BranchStatusLocal +} + +func (m *Manager) restoreManagedBranchFromRecoveryHeadLocked(ctx context.Context, wt *Worktree) error { + if wt == nil || wt.BranchOwner != BranchOwnerManaged || wt.RecoveryHeadSHA == "" { + return fmt.Errorf("managed branch recovery metadata is unavailable") + } + inspectCtx, cancel := context.WithTimeout(ctx, m.inspectTimeout) + defer cancel() + resolved, err := m.resolveCommit(inspectCtx, wt.RepositoryPath, wt.RecoveryHeadSHA) + if err != nil || resolved != strings.ToLower(wt.RecoveryHeadSHA) { + return fmt.Errorf("recovery commit is unavailable") + } + branchRef := "refs/heads/" + wt.Branch + zeroOID := strings.Repeat("0", len(resolved)) + cmd := m.newNonInteractiveGitCmd(inspectCtx, wt.RepositoryPath, "update-ref", branchRef, resolved, zeroOID) + if output, err := runGitCmdCombinedOutput(inspectCtx, cmd); err != nil { + return fmt.Errorf("restore managed branch: %s: %w", strings.TrimSpace(string(output)), err) + } + if err := m.finalizeRestoredManagedBranch(ctx, wt, resolved); err != nil { + return err + } + return nil +} + +func (m *Manager) finalizeRestoredManagedBranch(ctx context.Context, wt *Worktree, resolved string) error { + if wt.RecoveryHeadSHA == "" || !strings.EqualFold(wt.RecoveryHeadSHA, resolved) { + return fmt.Errorf("restored managed branch does not match its recovery head") + } + metadataStore, ok := m.store.(BranchMetadataStore) + if !ok { + return fmt.Errorf("persist restored compacted branch: metadata store is unavailable") + } + if !m.deleteRecoveryRef(ctx, wt, resolved) { + return fmt.Errorf("remove managed branch recovery ref") + } + persisted, persistErr := metadataStore.PersistBranchRecoveryRestored(ctx, wt.ID, resolved) + if persistErr != nil || !persisted { + return fmt.Errorf("persist restored compacted branch: %w", persistErr) + } + wt.RecoveryHeadSHA = "" + wt.BranchCompactedAt = nil + return nil +} + // refContains reports whether container already includes every commit in // contained, i.e. `git merge-base --is-ancestor contained container`. // A non-zero ancestry result is distinct from a failed probe: the former is a diff --git a/apps/backend/internal/worktree/manager_lifecycle.go b/apps/backend/internal/worktree/manager_lifecycle.go index b282566465..f376fffd1a 100644 --- a/apps/backend/internal/worktree/manager_lifecycle.go +++ b/apps/backend/internal/worktree/manager_lifecycle.go @@ -852,6 +852,7 @@ func (m *Manager) createInTaskDir(ctx context.Context, req CreateRequest, baseRe } wt := m.buildWorktreeRecord(worktreeID, req, worktreePath, branchName) + wt.BranchOwner = createdBranchOwner(checkoutMode.CheckoutBranch, branchName) if fetchResult != nil { wt.FetchWarning = fetchResult.Warning wt.FetchWarningDetail = fetchResult.WarningDetail @@ -907,6 +908,7 @@ func (m *Manager) createContributionInTaskDir( return nil, err } wt := m.buildWorktreeRecord(worktreeID, req, worktreePath, branchName) + wt.BranchOwner = BranchOwnerExternal if err := m.persistAndCacheWorktree(ctx, wt, req, worktreePath); err != nil { return nil, err } @@ -2054,18 +2056,43 @@ func (m *Manager) recreate(ctx context.Context, existing *Worktree, req CreateRe return nil, err } - // Archive deletes the local branch (removeWorktree runs `git branch -D`), - // so a recreate after unarchive must restore it first. fetchBranchToLocal - // fetches origin : — or pull//head for fork PRs, whose - // head branch never exists on origin by name — and only errors when the - // branch exists neither locally nor on the remote. - exists, probeErr := m.branchExists(ctx, req.RepositoryPath, existing.Branch) + // Archive may safely compact an integrated managed local branch, so a + // recreate after unarchive must restore its persisted exact head first. + // fetchBranchToLocal fetches origin : — or pull//head + // for fork PRs, whose head branch never exists on origin by name — and only + // errors when the branch exists neither locally nor on the remote. + // Probe the managed local ref explicitly. A short branch name can resolve + // through Git's DWIM rules to a remote-tracking ref, which must not preempt + // exact persisted recovery. + exists, probeErr := m.branchExists(ctx, req.RepositoryPath, "refs/heads/"+existing.Branch) if probeErr != nil { // "Could not tell" (timeout / fs stall) is not "missing" — reporting // ErrBranchUnrecoverable here would misclassify recoverable work as // gone. Propagate so the caller can retry the recreate. return nil, fmt.Errorf("cannot verify worktree branch %q: %w", existing.Branch, probeErr) } + if exists && existing.RecoveryHeadSHA != "" { + currentHead, resolveErr := m.resolveCommit(ctx, req.RepositoryPath, "refs/heads/"+existing.Branch) + if resolveErr != nil || !strings.EqualFold(currentHead, existing.RecoveryHeadSHA) { + return nil, fmt.Errorf("existing worktree branch %q does not match its recovery head", existing.Branch) + } + if restoreErr := m.finalizeRestoredManagedBranch(ctx, existing, currentHead); restoreErr != nil { + return nil, fmt.Errorf("persist restored compacted worktree branch %q: %w", existing.Branch, restoreErr) + } + } + // Recovery metadata is authoritative once the local ref is confirmed + // missing. Restore it before any refreshed-origin or contribution + // materialization can select a different head. + recoveredFromHead := false + if !exists { + existing.RepositoryPath = req.RepositoryPath + if restoreErr := m.restoreManagedBranchFromRecoveryHeadLocked(ctx, existing); restoreErr == nil { + exists = true + recoveredFromHead = true + } else if existing.BranchCompactedAt != nil { + return nil, fmt.Errorf("restore compacted worktree branch %q: %w", existing.Branch, restoreErr) + } + } contributionRemote := "" contributionRef := "" if req.RemoteContribution != nil { @@ -2076,11 +2103,8 @@ func (m *Manager) recreate(ctx context.Context, existing *Worktree, req CreateRe } } refreshedStartPoint := "" - if req.RemoteSyncHandled && req.RemoteContribution == nil && emptyRemoteBaseRef == "" { - sourceBranch := recreateSourceBranch(existing.Branch, req.CheckoutBranch) - selectedRef, prepareErr := m.prepareBranchFromRefreshedOrigin( - ctx, req.RepositoryPath, existing.Branch, sourceBranch, req.PRNumber, - ) + if shouldRefreshRecreatedBranch(req, recoveredFromHead, emptyRemoteBaseRef) { + sourceBranch, selectedRef, prepareErr := m.prepareRecreatedBranch(ctx, existing, req) if prepareErr != nil { return nil, prepareErr } @@ -2093,9 +2117,7 @@ func (m *Manager) recreate(ctx context.Context, existing *Worktree, req CreateRe ctx, existing, req, &BranchUnrecoverableError{Branch: sourceBranch}, ) } - if selectedRef != existing.Branch { - refreshedStartPoint = selectedRef - } + refreshedStartPoint = recreatedStartPoint(selectedRef, existing.Branch) exists = true } if !exists { @@ -2179,6 +2201,7 @@ func (m *Manager) recreate(ctx context.Context, existing *Worktree, req CreateRe existing.Path = worktreePath existing.Status = StatusActive existing.DeletedAt = nil + existing.BranchCompactedAt = nil existing.UpdatedAt = now existing.BaseBranchFallbackWarning = fallbackWarning existing.BaseBranchFallbackDetail = fallbackDetail @@ -2203,3 +2226,27 @@ func (m *Manager) recreate(ctx context.Context, existing *Worktree, req CreateRe return existing, nil } + +func (m *Manager) prepareRecreatedBranch( + ctx context.Context, existing *Worktree, req CreateRequest, +) (string, string, error) { + sourceBranch := existing.Branch + if req.CheckoutBranch != "" { + sourceBranch = req.CheckoutBranch + } + selectedRef, err := m.prepareBranchFromRefreshedOrigin( + ctx, req.RepositoryPath, existing.Branch, sourceBranch, req.PRNumber, + ) + return sourceBranch, selectedRef, err +} + +func recreatedStartPoint(selectedRef, existingBranch string) string { + if selectedRef == existingBranch { + return "" + } + return selectedRef +} + +func shouldRefreshRecreatedBranch(req CreateRequest, recoveredFromHead bool, emptyRemoteBaseRef string) bool { + return !recoveredFromHead && req.RemoteSyncHandled && req.RemoteContribution == nil && emptyRemoteBaseRef == "" +} diff --git a/apps/backend/internal/worktree/manager_recreate_recovery_test.go b/apps/backend/internal/worktree/manager_recreate_recovery_test.go index 5bffd93832..2ba7acf2b7 100644 --- a/apps/backend/internal/worktree/manager_recreate_recovery_test.go +++ b/apps/backend/internal/worktree/manager_recreate_recovery_test.go @@ -7,13 +7,13 @@ import ( "path/filepath" "strings" "testing" + "time" "github.com/kandev/kandev/internal/task/models" ) -// archiveDeletesLocalBranch simulates what task archive does to a worktree's -// branch: the local ref is deleted (`git branch -D`), while origin and the -// remote-tracking ref are left alone. +// archiveDeletesLocalBranch simulates a compacted archived worktree branch: the +// local ref is absent while origin and the remote-tracking ref are left alone. func archiveDeletesLocalBranch(t *testing.T, repoPath, branch string) { t.Helper() runGit(t, repoPath, "branch", "-D", branch) @@ -515,6 +515,146 @@ func TestRecreate_ManagedRefreshUsesRemoteWhenLocalCheckoutBranchIsBehind(t *tes } } +func TestRecreate_RecoveryHeadWinsOverRefreshedRemoteWhenSyncHandled(t *testing.T) { + repoPath := initGitRepoWithRemote(t) + recoverySHA := strings.TrimSpace(runGit(t, repoPath, "rev-parse", "feature/pr-branch")) + refreshedSHA := advanceRemoteBranch(t, repoPath, "feature/pr-branch") + if refreshedSHA == recoverySHA { + t.Fatal("remote refresh did not advance the branch") + } + archiveDeletesLocalBranch(t, repoPath, "feature/pr-branch") + worktreePath := filepath.Join(t.TempDir(), "task-recovery-precedence", "repo-1") + if err := os.MkdirAll(worktreePath, 0755); err != nil { + t.Fatalf("create worktree placeholder: %v", err) + } + + store := newMockStore() + mgr, err := NewManager(newTestConfig(t), store, newTestLogger()) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + existing := &Worktree{ + ID: "wt-recovery-precedence", SessionID: "session-recovery-precedence", + TaskID: "task-recovery-precedence", RepositoryID: "repo-1", RepositoryPath: repoPath, + Path: worktreePath, Branch: "feature/pr-branch", BranchOwner: BranchOwnerManaged, + RecoveryHeadSHA: recoverySHA, Status: StatusDeleted, + } + store.worktrees[existing.ID] = existing + wt, err := mgr.recreate(context.Background(), existing, CreateRequest{ + SessionID: "session-recovery-precedence", TaskID: "task-recovery-precedence", RepositoryID: "repo-1", + RepositoryPath: repoPath, CheckoutBranch: "feature/pr-branch", RemoteSyncHandled: true, + }) + if err != nil { + t.Fatalf("recreate(): %v", err) + } + if got := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD")); got != recoverySHA { + t.Fatalf("restored HEAD = %q, want persisted recovery SHA %q (refreshed remote was %q)", got, recoverySHA, refreshedSHA) + } +} + +// TestRecreate_CompactedBranchWithUnavailableRecoveryRefRefusesFallback pins +// the fail-closed boundary: an archived compaction marker cannot fall back to +// an advanced base or remote branch when its exact recovery object is gone. +func TestRecreate_CompactedBranchWithUnavailableRecoveryRefRefusesFallback(t *testing.T) { + repoPath := initGitRepoWithRemote(t) + advanceRemoteBranch(t, repoPath, "main") + advanceRemoteBranch(t, repoPath, "feature/pr-branch") + archiveDeletesLocalBranch(t, repoPath, "feature/pr-branch") + + worktreePath := filepath.Join(t.TempDir(), "task-compacted-missing-recovery", "repo-1") + if err := os.MkdirAll(worktreePath, 0o755); err != nil { + t.Fatalf("create worktree placeholder: %v", err) + } + compactedAt := time.Now().UTC() + mgr := newRecreateTestManager(t) + _, err := mgr.recreate(context.Background(), &Worktree{ + ID: "wt-compacted-missing-recovery", SessionID: "session-compacted-missing-recovery", + TaskID: "task-compacted-missing-recovery", RepositoryID: "repo-1", RepositoryPath: repoPath, + Path: worktreePath, Branch: "feature/pr-branch", BranchOwner: BranchOwnerManaged, + RecoveryHeadSHA: strings.Repeat("f", 40), BranchCompactedAt: &compactedAt, Status: StatusDeleted, + }, CreateRequest{ + SessionID: "session-compacted-missing-recovery", TaskID: "task-compacted-missing-recovery", + RepositoryID: "repo-1", RepositoryPath: repoPath, BaseBranch: "main", + FallbackBaseBranch: "main", CheckoutBranch: "feature/pr-branch", PullBeforeWorktree: true, + }) + if err == nil || !strings.Contains(err.Error(), "restore compacted worktree branch") { + t.Fatalf("recreate() error = %v, want fail-closed recovery error", err) + } + if _, statErr := os.Stat(worktreePath); !errors.Is(statErr, os.ErrNotExist) { + t.Fatalf("fallback recreated worktree path = %q (%v), want no fallback", worktreePath, statErr) + } + if got := mgr.BranchRecoveryStatus(context.Background(), repoPath, "feature/pr-branch"); got != BranchStatusRemote { + t.Fatalf("recovery status = %q, want remote-only branch after refusal", got) + } +} + +func TestRestoreManagedBranchFromRecoveryHead_RefusesConcurrentCreator(t *testing.T) { + repoPath := initGitRepoWithRemote(t) + recoverySHA := strings.TrimSpace(runGit(t, repoPath, "rev-parse", "feature/pr-branch")) + creatorSHA := strings.TrimSpace(runGit(t, repoPath, "rev-parse", "main")) + runGit(t, repoPath, "branch", "feature/recovery-cas", creatorSHA) + + mgr := newRecreateTestManager(t) + err := mgr.restoreManagedBranchFromRecoveryHeadLocked(context.Background(), &Worktree{ + RepositoryPath: repoPath, Branch: "feature/recovery-cas", BranchOwner: BranchOwnerManaged, + RecoveryHeadSHA: recoverySHA, + }) + if err == nil { + t.Fatal("restore succeeded despite an existing branch ref") + } + if got := strings.TrimSpace(runGit(t, repoPath, "rev-parse", "refs/heads/feature/recovery-cas")); got != creatorSHA { + t.Fatalf("concurrent creator branch changed to %q, want %q", got, creatorSHA) + } +} + +func TestRecoverBranchStatus_RejectsReusedBranchRefWithDifferentRecoveryHead(t *testing.T) { + repoPath := initGitRepoWithRemote(t) + recoverySHA := strings.TrimSpace(runGit(t, repoPath, "rev-parse", "feature/pr-branch")) + runGit(t, repoPath, "checkout", "main") + runGit(t, repoPath, "commit", "--allow-empty", "-m", "unrelated branch reuse") + unrelatedSHA := strings.TrimSpace(runGit(t, repoPath, "rev-parse", "HEAD")) + runGit(t, repoPath, "update-ref", "refs/heads/feature/pr-branch", unrelatedSHA, recoverySHA) + + mgr := newRecreateTestManager(t) + status := mgr.RecoverBranchStatus(context.Background(), &Worktree{ + RepositoryPath: repoPath, Branch: "feature/pr-branch", BranchOwner: BranchOwnerManaged, + RecoveryHeadSHA: recoverySHA, + }) + if status != BranchStatusMissing { + t.Fatalf("recovery status = %q, want fail-closed missing", status) + } + if got := strings.TrimSpace(runGit(t, repoPath, "rev-parse", "refs/heads/feature/pr-branch")); got != unrelatedSHA { + t.Fatalf("reused branch changed to %q, want %q", got, unrelatedSHA) + } +} + +func TestRecreate_RejectsReusedBranchRefWithDifferentRecoveryHead(t *testing.T) { + repoPath := initGitRepoWithRemote(t) + recoverySHA := strings.TrimSpace(runGit(t, repoPath, "rev-parse", "feature/pr-branch")) + runGit(t, repoPath, "checkout", "main") + runGit(t, repoPath, "commit", "--allow-empty", "-m", "unrelated recreate reuse") + unrelatedSHA := strings.TrimSpace(runGit(t, repoPath, "rev-parse", "HEAD")) + runGit(t, repoPath, "update-ref", "refs/heads/feature/pr-branch", unrelatedSHA, recoverySHA) + + mgr := newRecreateTestManager(t) + _, err := mgr.recreate(context.Background(), &Worktree{ + ID: "wt-reused-ref", SessionID: "session-reused-ref", TaskID: "task-reused-ref", + RepositoryID: "repo-1", RepositoryPath: repoPath, + Path: filepath.Join(t.TempDir(), "task-reused-ref", "repo-1"), + Branch: "feature/pr-branch", BranchOwner: BranchOwnerManaged, RecoveryHeadSHA: recoverySHA, + Status: StatusDeleted, + }, CreateRequest{ + SessionID: "session-reused-ref", TaskID: "task-reused-ref", RepositoryID: "repo-1", + RepositoryPath: repoPath, BaseBranch: "main", FallbackBaseBranch: "main", + }) + if err == nil || !strings.Contains(err.Error(), "does not match its recovery head") { + t.Fatalf("recreate error = %v, want reused-ref rejection", err) + } + if got := strings.TrimSpace(runGit(t, repoPath, "rev-parse", "refs/heads/feature/pr-branch")); got != unrelatedSHA { + t.Fatalf("reused branch changed to %q, want %q", got, unrelatedSHA) + } +} + func TestRecreate_ManagedRefreshUsesRemotePRHeadWhenLocalCheckoutBranchIsBehind(t *testing.T) { repoPath, wantSHA := initManagedPRCheckoutBranch(t, 977, "feature/managed-fork-pr-behind") runGit(t, repoPath, "remote", "set-url", "origin", "https://127.0.0.1:1/never.git") @@ -570,13 +710,14 @@ func TestCreate_RestoresReleasedWorktreeAfterArchive(t *testing.T) { if err != nil { t.Fatalf("create worktree: %v", err) } - // Work the user never pushed. It only survives because archive keeps the - // branch (DestroyWorktree passes removeBranch=false). + // Work the user never pushed. It survives because ancestry validation + // retains every branch with commits outside the integration ref. runGit(t, wt.Path, "commit", "--allow-empty", "-m", "unpushed work") workSHA := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD")) - // Archive: remove the directory, release the reference, keep the branch. - if err := mgr.RemoveByID(ctx, wt.ID, false); err != nil { + // Archive: remove the directory and release the reference; the unique + // commit makes the branch ineligible for compaction. + if err := mgr.CleanupWorktreesPreservingBranches(ctx, []*Worktree{wt}); err != nil { t.Fatalf("archive worktree: %v", err) } if _, statErr := os.Stat(wt.Path); !os.IsNotExist(statErr) { @@ -614,3 +755,60 @@ func TestCreate_RestoresReleasedWorktreeAfterArchive(t *testing.T) { } assertWorktreeReferenceStatus(t, store, wt.ID, StatusActive) } + +func TestCreate_RestoresSafelyCompactedBranchFromExactHead(t *testing.T) { + mgr, store := newReferenceCleanupTestManager(t) + ctx := context.Background() + seedReferenceCleanupSession(t, store, "task-integrated", "session-integrated", models.TaskSessionStateCompleted) + req := CreateRequest{ + TaskID: "task-integrated", + SessionID: "session-integrated", + TaskTitle: "Integrated work", + RepositoryID: "repository", + RepositoryPath: initGitRepoWithRemote(t), + BaseBranch: "main", + IntegrationRef: "main", + TaskDirName: "task-integrated", + RepoName: "repository", + } + wt, err := mgr.Create(ctx, req) + if err != nil { + t.Fatalf("create worktree: %v", err) + } + wantSHA := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD")) + + if err := mgr.CleanupWorktreesPreservingBranches(ctx, []*Worktree{wt}); err != nil { + t.Fatalf("archive worktree: %v", err) + } + if got := strings.TrimSpace(runGit(t, wt.RepositoryPath, "branch", "--list", wt.Branch)); got != "" { + t.Fatalf("fully integrated branch was not compacted: %q", got) + } + archived, err := store.GetWorktreeByID(ctx, wt.ID) + if err != nil { + t.Fatalf("load compacted worktree metadata: %v", err) + } + if archived.BranchOwner != BranchOwnerManaged || archived.RecoveryHeadSHA != wantSHA { + t.Fatalf("compacted recovery metadata = owner %q head %q, want %q at %q", + archived.BranchOwner, archived.RecoveryHeadSHA, BranchOwnerManaged, wantSHA) + } + + resumeReq := req + resumeReq.WorktreeID = wt.ID + restored, err := mgr.Create(ctx, resumeReq) + if err != nil { + t.Fatalf("recreate compacted worktree: %v", err) + } + if got := strings.TrimSpace(runGit(t, restored.Path, "rev-parse", "HEAD")); got != wantSHA { + t.Fatalf("restored HEAD = %q, want exact compacted head %q", got, wantSHA) + } + persistedRestored, err := store.GetWorktreeByID(ctx, restored.ID) + if err != nil { + t.Fatalf("load restored worktree: %v", err) + } + if persistedRestored.RecoveryHeadSHA != "" { + t.Fatalf("restored worktree kept recovery head %q, want it cleared", persistedRestored.RecoveryHeadSHA) + } + if persistedRestored.BranchCompactedAt != nil { + t.Fatalf("restored worktree kept compaction marker %v", persistedRestored.BranchCompactedAt) + } +} diff --git a/apps/backend/internal/worktree/manager_state.go b/apps/backend/internal/worktree/manager_state.go index 38741fa4f5..5311009c85 100644 --- a/apps/backend/internal/worktree/manager_state.go +++ b/apps/backend/internal/worktree/manager_state.go @@ -28,6 +28,7 @@ func (m *Manager) buildWorktreeRecord(worktreeID string, req CreateRequest, work Path: worktreePath, Branch: branchName, BaseBranch: req.BaseBranch, + IntegrationRef: req.IntegrationRef, Status: StatusActive, CreatedAt: now, UpdatedAt: now, diff --git a/apps/backend/internal/worktree/manager_test.go b/apps/backend/internal/worktree/manager_test.go index f0b8d0ff36..3dce38dc83 100644 --- a/apps/backend/internal/worktree/manager_test.go +++ b/apps/backend/internal/worktree/manager_test.go @@ -118,6 +118,47 @@ func (s *mockStore) CountActiveWorktreeReferences(_ context.Context, _ string, _ return 0, nil } +func (s *mockStore) CountWorktreeBranchOwners(_ context.Context, repositoryPath, branch string) (int, error) { + count := 0 + for _, wt := range s.worktrees { + if wt.RepositoryPath == repositoryPath && wt.Branch == branch { + count++ + } + } + return count, nil +} + +func (s *mockStore) PersistBranchRecoveryHead(_ context.Context, worktreeID, expected, recoveryHead string) (bool, error) { + wt := s.worktrees[worktreeID] + if wt == nil || (wt.RecoveryHeadSHA != expected && wt.RecoveryHeadSHA != recoveryHead) { + return false, nil + } + wt.RecoveryHeadSHA = recoveryHead + return true, nil +} + +func (s *mockStore) PersistBranchCompactionComplete( + _ context.Context, worktreeID, expectedRecoveryHead string, +) (bool, error) { + wt := s.worktrees[worktreeID] + if wt == nil || wt.RecoveryHeadSHA != expectedRecoveryHead || wt.BranchCompactedAt != nil { + return false, nil + } + now := time.Now().UTC() + wt.BranchCompactedAt = &now + return true, nil +} + +func (s *mockStore) PersistBranchRecoveryRestored(_ context.Context, worktreeID, expectedRecoveryHead string) (bool, error) { + wt := s.worktrees[worktreeID] + if wt == nil || wt.RecoveryHeadSHA != expectedRecoveryHead { + return false, nil + } + wt.RecoveryHeadSHA = "" + wt.BranchCompactedAt = nil + return true, nil +} + // GetWorktreesBySessionID — MultiRepoStore. func (s *mockStore) GetWorktreesBySessionID(_ context.Context, sessionID string) ([]*Worktree, error) { var out []*Worktree diff --git a/apps/backend/internal/worktree/manager_timeout_test.go b/apps/backend/internal/worktree/manager_timeout_test.go index daf9c61605..39a1d8e12a 100644 --- a/apps/backend/internal/worktree/manager_timeout_test.go +++ b/apps/backend/internal/worktree/manager_timeout_test.go @@ -3,9 +3,15 @@ package worktree import ( "context" "errors" + "expvar" "os" + "path/filepath" + "strconv" + "strings" "testing" "time" + + "github.com/kandev/kandev/internal/common/subproc" ) // hangOnRevParseScript is the shared fake-git script body used by the timeout @@ -24,6 +30,32 @@ case "${1:-}" in esac ` +const hangOnUpdateRefScript = ` +case "${1:-}" in + update-ref) + sleep 30 + exit 0 + ;; + *) + exit 0 + ;; +esac +` + +type boundedBranchMetadataStore struct{ mockStore } + +func (s *boundedBranchMetadataStore) CountWorktreeBranchOwners(context.Context, string, string) (int, error) { + return 1, nil +} + +func (s *boundedBranchMetadataStore) PersistBranchRecoveryHead(context.Context, string, string, string) (bool, error) { + return true, nil +} + +func (s *boundedBranchMetadataStore) PersistBranchCompactionComplete(context.Context, string, string) (bool, error) { + return true, nil +} + // TestBranchExists_RespectsContextDeadline verifies that branchExists cancels // the underlying git subprocess when its caller-provided context expires. func TestBranchExists_RespectsContextDeadline(t *testing.T) { @@ -113,6 +145,185 @@ func TestBranchExists_BoundedWhenCallerHasNoDeadline(t *testing.T) { } } +// TestResolveCommit_BoundedWhenCallerHasNoDeadline verifies that commit +// probes used by locked cleanup cannot leave a repository lock held by a +// stalled Git process. +func TestResolveCommit_BoundedWhenCallerHasNoDeadline(t *testing.T) { + scriptDir := writeFakeGitScript(t, hangOnRevParseScript) + t.Setenv("PATH", scriptDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + mgr, err := NewManager(newTestConfig(t), newMockStore(), newTestLogger()) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + mgr.inspectTimeout = 300 * time.Millisecond + + start := time.Now() + commit, err := mgr.resolveCommit(context.Background(), t.TempDir(), "refs/heads/main") + elapsed := time.Since(start) + + if commit != "" { + t.Fatalf("resolveCommit() = %q, want empty on hanging git", commit) + } + if err == nil { + t.Fatal("resolveCommit() err = nil, want timeout error") + } + if elapsed > 2*time.Second { + t.Fatalf("resolveCommit() took %v, want <2s (inspect timeout not applied)", elapsed) + } +} + +func TestDeleteExpectedBranchRef_BoundedWhenUpdateRefHangs(t *testing.T) { + scriptDir := writeFakeGitScript(t, hangOnUpdateRefScript) + t.Setenv("PATH", scriptDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + store := &boundedBranchMetadataStore{mockStore: *newMockStore()} + mgr, err := NewManager(newTestConfig(t), store, newTestLogger()) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + mgr.inspectTimeout = 300 * time.Millisecond + + wt := &Worktree{ID: "wt-timeout", RepositoryPath: t.TempDir(), Branch: "feature/timeout"} + start := time.Now() + reason := mgr.deleteExpectedBranchRef(context.Background(), store, wt, strings.Repeat("a", 40)) + elapsed := time.Since(start) + + if reason != RetainedSafeDeleteRefused { + t.Fatalf("deleteExpectedBranchRef() reason = %q, want %q", reason, RetainedSafeDeleteRefused) + } + if elapsed > 2*time.Second { + t.Fatalf("deleteExpectedBranchRef() took %v, want <2s", elapsed) + } +} + +func TestRestoreManagedBranchFromRecoveryHead_BoundsUpdateRefAdmissionAndReleasesRepoLock(t *testing.T) { + restoreCap := setGitThrottleCapForTest(2) + t.Cleanup(restoreCap) + + recoverySHA := strings.Repeat("a", 40) + barrierDir := t.TempDir() + revParseStarted := filepath.Join(barrierDir, "rev-parse-started") + allowRevParse := filepath.Join(barrierDir, "allow-rev-parse") + scriptDir := writeFakeGitScript(t, ` +case "${1:-}" in + rev-parse) + : > "`+revParseStarted+`" + while [ ! -f "`+allowRevParse+`" ]; do sleep 0.01; done + printf '%s\n' "`+recoverySHA+`" + ;; + update-ref) + exit 0 + ;; + *) + exit 0 + ;; +esac +`) + t.Setenv("PATH", scriptDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + mgr, err := NewManager(newTestConfig(t), newMockStore(), newTestLogger()) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + mgr.inspectTimeout = 300 * time.Millisecond + repoPath := t.TempDir() + repoLock := mgr.getRepoLock(repoPath) + firstHold, err := subproc.AcquireGit(context.Background(), subproc.GitLifecycle) + if err != nil { + t.Fatalf("hold first lifecycle slot: %v", err) + } + t.Cleanup(firstHold) + + resultCh := make(chan error, 1) + go func() { + repoLock.Lock() + err := mgr.restoreManagedBranchFromRecoveryHeadLocked(context.Background(), &Worktree{ + RepositoryPath: repoPath, + Branch: "feature/recovery-timeout", + BranchOwner: BranchOwnerManaged, + RecoveryHeadSHA: recoverySHA, + }) + repoLock.Unlock() + resultCh <- err + }() + waitForFile(t, revParseStarted) + + secondHoldCh := make(chan func(), 1) + secondHoldErrCh := make(chan error, 1) + go func() { + release, acquireErr := subproc.AcquireGit(context.Background(), subproc.GitLifecycle) + if acquireErr != nil { + secondHoldErrCh <- acquireErr + return + } + secondHoldCh <- release + }() + waitForGitLifecycleWaiters(t, 1) + if err := os.WriteFile(allowRevParse, []byte("go"), 0o600); err != nil { + t.Fatalf("release rev-parse barrier: %v", err) + } + var secondHold func() + select { + case err := <-secondHoldErrCh: + t.Fatalf("hold second lifecycle slot: %v", err) + case secondHold = <-secondHoldCh: + case <-time.After(time.Second): + t.Fatal("second lifecycle slot was not acquired") + } + t.Cleanup(secondHold) + + select { + case err := <-resultCh: + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("restore error = %v, want bounded update-ref admission deadline", err) + } + case <-time.After(2 * time.Second): + t.Fatal("restore remained blocked on saturated update-ref admission") + } + if !repoLock.TryLock() { + t.Fatal("repository lock remained held after bounded recovery restore") + } + repoLock.Unlock() +} + +func waitForFile(t *testing.T, path string) { + t.Helper() + deadline := time.Now().Add(time.Second) + for time.Now().Before(deadline) { + if _, err := os.Stat(path); err == nil { + return + } + time.Sleep(10 * time.Millisecond) + } + t.Fatalf("file %s was not created", path) +} + +func waitForGitLifecycleWaiters(t *testing.T, want int64) { + t.Helper() + deadline := time.Now().Add(time.Second) + for time.Now().Before(deadline) { + if got := gitLifecycleWaiters(); got == want { + return + } + time.Sleep(10 * time.Millisecond) + } + t.Fatalf("git lifecycle waiters did not reach %d (got %d)", want, gitLifecycleWaiters()) +} + +func gitLifecycleWaiters() int64 { + vars, ok := expvar.Get("subproc_class_waiters").(*expvar.Map) + if !ok { + return 0 + } + value := vars.Get("pool=git;class=lifecycle") + if value == nil { + return 0 + } + got, _ := strconv.ParseInt(value.String(), 10, 64) + return got +} + // TestCreate_HangingRevParseReleasesRepoLock is the core regression test for // the reported symptom: a hung git rev-parse during Create must not keep the // per-repo mutex locked indefinitely. Before the fix, branchExists and diff --git a/apps/backend/internal/worktree/repository_adapter.go b/apps/backend/internal/worktree/repository_adapter.go index 0dfe003278..188c65f478 100644 --- a/apps/backend/internal/worktree/repository_adapter.go +++ b/apps/backend/internal/worktree/repository_adapter.go @@ -9,6 +9,7 @@ import ( // Repository contains repository information needed for script execution. type Repository struct { ID string + DefaultBranch string SetupScript string CleanupScript string CopyFiles string @@ -45,6 +46,7 @@ func (a *RepositoryAdapter) GetRepository(ctx context.Context, repositoryID stri return &Repository{ ID: repo.ID, + DefaultBranch: repo.DefaultBranch, SetupScript: repo.SetupScript, CleanupScript: repo.CleanupScript, CopyFiles: repo.CopyFiles, diff --git a/apps/backend/internal/worktree/store.go b/apps/backend/internal/worktree/store.go index b6ce89d959..2c205186c3 100644 --- a/apps/backend/internal/worktree/store.go +++ b/apps/backend/internal/worktree/store.go @@ -66,6 +66,10 @@ const worktreeSelectCols = ` ter.worktree_branch, COALESCE(ter.branch_slug, ''), COALESCE(s.base_branch, ''), + COALESCE(ter.worktree_branch_owner, 'unknown'), + COALESCE(ter.worktree_integration_ref, ''), + COALESCE(ter.worktree_recovery_head_sha, ''), + ter.worktree_branch_compacted_at, ter.status, ter.created_at, ter.updated_at, @@ -82,7 +86,7 @@ type rowScanner interface { // scanWorktreeRow scans one environment-repository row into a Worktree value. func scanWorktreeRow(row rowScanner) (*Worktree, error) { wt := &Worktree{} - var mergedAt, deletedAt sql.NullTime + var compactedAt, mergedAt, deletedAt sql.NullTime var repositoryPath, baseBranch sql.NullString err := row.Scan( @@ -96,6 +100,10 @@ func scanWorktreeRow(row rowScanner) (*Worktree, error) { &wt.Branch, &wt.BranchSlug, &baseBranch, + &wt.BranchOwner, + &wt.IntegrationRef, + &wt.RecoveryHeadSHA, + &compactedAt, &wt.Status, &wt.CreatedAt, &wt.UpdatedAt, @@ -119,6 +127,10 @@ func scanWorktreeRow(row rowScanner) (*Worktree, error) { if mergedAt.Valid { wt.MergedAt = &mergedAt.Time } + if compactedAt.Valid { + t := compactedAt.Time + wt.BranchCompactedAt = &t + } if deletedAt.Valid { wt.DeletedAt = &deletedAt.Time } @@ -172,6 +184,10 @@ func (s *SQLiteStore) CreateWorktree(ctx context.Context, wt *Worktree) error { wt.Status = StatusActive } now := time.Now().UTC() + // A materialized worktree means the local branch is present again. Clear + // the prior compaction completion marker so a later archive can be + // evaluated independently after new commits. + wt.BranchCompactedAt = nil if wt.CreatedAt.IsZero() { wt.CreatedAt = now } @@ -201,19 +217,26 @@ func (s *SQLiteStore) CreateWorktree(ctx context.Context, wt *Worktree) error { _, err = tx.ExecContext(ctx, s.db.Rebind(` INSERT INTO task_environment_repos ( id, task_environment_id, repository_id, branch_slug, - worktree_id, worktree_path, worktree_branch, position, + worktree_id, worktree_path, worktree_branch, + worktree_branch_owner, worktree_integration_ref, worktree_recovery_head_sha, + worktree_branch_compacted_at, position, error_message, status, created_at, updated_at, merged_at, deleted_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(task_environment_id, repository_id, branch_slug) DO UPDATE SET worktree_id = excluded.worktree_id, worktree_path = excluded.worktree_path, worktree_branch = excluded.worktree_branch, + worktree_branch_owner = excluded.worktree_branch_owner, + worktree_integration_ref = excluded.worktree_integration_ref, + worktree_recovery_head_sha = excluded.worktree_recovery_head_sha, + worktree_branch_compacted_at = excluded.worktree_branch_compacted_at, status = excluded.status, updated_at = excluded.updated_at, merged_at = excluded.merged_at, deleted_at = excluded.deleted_at `), uuid.New().String(), envID, wt.RepositoryID, wt.BranchSlug, - wt.ID, wt.Path, wt.Branch, 0, + wt.ID, wt.Path, wt.Branch, wt.BranchOwner, wt.IntegrationRef, wt.RecoveryHeadSHA, + wt.BranchCompactedAt, 0, "", wt.Status, wt.CreatedAt, wt.UpdatedAt, wt.MergedAt, wt.DeletedAt) if err != nil { @@ -439,12 +462,18 @@ func (s *SQLiteStore) UpdateWorktree(ctx context.Context, wt *Worktree) error { query := ` UPDATE task_environment_repos SET worktree_path = ?, worktree_branch = ?, + worktree_branch_owner = ?, worktree_integration_ref = ?, worktree_recovery_head_sha = ?, + worktree_branch_compacted_at = ?, status = ?, updated_at = ?, merged_at = ?, deleted_at = ? WHERE worktree_id = ? ` args := []interface{}{ wt.Path, wt.Branch, + wt.BranchOwner, + wt.IntegrationRef, + wt.RecoveryHeadSHA, + wt.BranchCompactedAt, wt.Status, wt.UpdatedAt, wt.MergedAt, @@ -585,6 +614,216 @@ func (s *SQLiteStore) CountActiveWorktreeReferences( return count, err } +func (s *SQLiteStore) CountWorktreeBranchOwners( + ctx context.Context, repositoryPath, branch string, +) (int, error) { + // Count every durable claimant, including soft-deleted rows. A deleted row + // may still retain the local ref, so ambiguity must fail closed. + var count int + err := s.ro.QueryRowContext(ctx, s.ro.Rebind(` + SELECT COUNT(*) + FROM task_environment_repos ter + LEFT JOIN repositories r ON r.id = ter.repository_id + WHERE (r.local_path = ? OR (r.local_path IS NULL AND NOT EXISTS ( + SELECT 1 FROM repositories r2 WHERE r2.local_path = ? AND r2.local_path <> '' + ))) AND ter.worktree_branch = ? + AND COALESCE(worktree_id, '') <> '' + `), repositoryPath, repositoryPath, branch).Scan(&count) + return count, err +} + +func (s *SQLiteStore) PersistBranchRecoveryHead( + ctx context.Context, worktreeID, expected, recoveryHead string, +) (bool, error) { + result, err := s.db.ExecContext(ctx, s.db.Rebind(` + UPDATE task_environment_repos + SET worktree_recovery_head_sha = ?, updated_at = ? + WHERE worktree_id = ? + AND (COALESCE(worktree_recovery_head_sha, '') = ? OR worktree_recovery_head_sha = ?) + `), recoveryHead, time.Now().UTC(), worktreeID, expected, recoveryHead) + if err != nil { + return false, err + } + rows, err := result.RowsAffected() + return rows == 1, err +} + +func (s *SQLiteStore) PersistBranchCompactionComplete( + ctx context.Context, worktreeID, expectedRecoveryHead string, +) (bool, error) { + now := time.Now().UTC() + result, err := s.db.ExecContext(ctx, s.db.Rebind(` + UPDATE task_environment_repos + SET worktree_branch_compacted_at = ?, updated_at = ? + WHERE worktree_id = ? + AND worktree_recovery_head_sha = ? + AND worktree_branch_compacted_at IS NULL + `), now, now, worktreeID, expectedRecoveryHead) + if err != nil { + return false, err + } + rows, err := result.RowsAffected() + return rows == 1, err +} + +// PersistBranchRecoveryRestored clears the exact recovery state after the local +// branch is confirmed at its recorded head and the recovery ref is removed. +// It also finalizes an interrupted compaction that never recorded completion. +func (s *SQLiteStore) PersistBranchRecoveryRestored( + ctx context.Context, worktreeID, expectedRecoveryHead string, +) (bool, error) { + result, err := s.db.ExecContext(ctx, s.db.Rebind(` + UPDATE task_environment_repos + SET worktree_recovery_head_sha = '', worktree_branch_compacted_at = NULL, updated_at = ? + WHERE worktree_id = ? + AND worktree_recovery_head_sha = ? + `), time.Now().UTC(), worktreeID, expectedRecoveryHead) + if err != nil { + return false, err + } + rows, err := result.RowsAffected() + return rows == 1, err +} + +func (s *SQLiteStore) ListArchivedBranchCandidates( + ctx context.Context, limit int, +) ([]*Worktree, error) { + rows, err := s.ro.QueryContext(ctx, s.ro.Rebind(` + SELECT `+worktreeSelectCols+` + FROM task_environment_repos ter + INNER JOIN task_environments te ON ter.task_environment_id = te.id + INNER JOIN tasks t ON te.task_id = t.id + LEFT JOIN task_sessions s ON s.task_environment_id = ter.task_environment_id + LEFT JOIN repositories r ON ter.repository_id = r.id + WHERE t.archived_at IS NOT NULL + AND ter.status = ? + AND ter.deleted_at IS NOT NULL + AND ter.worktree_branch_owner = ? + AND ter.worktree_branch_compacted_at IS NULL + AND COALESCE(ter.worktree_id, '') <> '' + AND NOT EXISTS ( + SELECT 1 FROM task_sessions active_session + WHERE active_session.task_id = t.id + AND active_session.state IN (?, ?, ?, ?) + ) + ORDER BY ter.updated_at ASC, ter.worktree_id ASC + LIMIT ? + `), StatusDeleted, BranchOwnerManaged, + models.TaskSessionStateCreated, models.TaskSessionStateStarting, + models.TaskSessionStateRunning, models.TaskSessionStateWaitingForInput, + limit) + if err != nil { + return nil, err + } + defer func() { _ = rows.Close() }() + return s.scanWorktrees(rows) +} + +func (s *SQLiteStore) IsArchivedBranchCandidate(ctx context.Context, worktreeID string) (bool, error) { + var eligible bool + err := s.ro.QueryRowContext(ctx, s.ro.Rebind(` + SELECT EXISTS ( + SELECT 1 + FROM task_environment_repos ter + INNER JOIN task_environments te ON ter.task_environment_id = te.id + INNER JOIN tasks t ON te.task_id = t.id + WHERE ter.worktree_id = ? + AND t.archived_at IS NOT NULL + AND ter.status = ? + AND ter.deleted_at IS NOT NULL + AND ter.worktree_branch_owner = ? + AND ter.worktree_branch_compacted_at IS NULL + AND NOT EXISTS ( + SELECT 1 FROM task_sessions active_session + WHERE active_session.task_id = t.id + AND active_session.state IN (?, ?, ?, ?) + ) + ) + `), worktreeID, StatusDeleted, BranchOwnerManaged, + models.TaskSessionStateCreated, models.TaskSessionStateStarting, + models.TaskSessionStateRunning, models.TaskSessionStateWaitingForInput).Scan(&eligible) + return eligible, err +} + +func (s *SQLiteStore) PersistArchivedBranchRecoveryHead( + ctx context.Context, worktreeID, expected, recoveryHead string, +) (bool, error) { + result, err := s.db.ExecContext(ctx, s.db.Rebind(` + UPDATE task_environment_repos + SET worktree_recovery_head_sha = ?, updated_at = ? + WHERE worktree_id = ? + AND status = ? + AND deleted_at IS NOT NULL + AND worktree_branch_owner = ? + AND worktree_branch_compacted_at IS NULL + AND (COALESCE(worktree_recovery_head_sha, '') = ? OR worktree_recovery_head_sha = ?) + AND EXISTS ( + SELECT 1 + FROM task_environments te + INNER JOIN tasks t ON te.task_id = t.id + WHERE te.id = task_environment_repos.task_environment_id + AND t.archived_at IS NOT NULL + AND NOT EXISTS ( + SELECT 1 FROM task_sessions active_session + WHERE active_session.task_id = t.id + AND active_session.state IN (?, ?, ?, ?) + ) + ) + `), recoveryHead, time.Now().UTC(), worktreeID, StatusDeleted, BranchOwnerManaged, + expected, recoveryHead, + models.TaskSessionStateCreated, models.TaskSessionStateStarting, + models.TaskSessionStateRunning, models.TaskSessionStateWaitingForInput) + if err != nil { + return false, err + } + rows, err := result.RowsAffected() + return rows == 1, err +} + +func (s *SQLiteStore) PersistArchivedBranchCompactionComplete( + ctx context.Context, worktreeID, expectedRecoveryHead string, +) (bool, error) { + now := time.Now().UTC() + result, err := s.db.ExecContext(ctx, s.db.Rebind(` + UPDATE task_environment_repos + SET worktree_branch_compacted_at = ?, updated_at = ? + WHERE worktree_id = ? + AND status = ? + AND deleted_at IS NOT NULL + AND worktree_branch_owner = ? + AND worktree_recovery_head_sha = ? + AND worktree_branch_compacted_at IS NULL + AND EXISTS ( + SELECT 1 + FROM task_environments te + INNER JOIN tasks t ON te.task_id = t.id + WHERE te.id = task_environment_repos.task_environment_id + AND t.archived_at IS NOT NULL + AND NOT EXISTS ( + SELECT 1 FROM task_sessions active_session + WHERE active_session.task_id = t.id + AND active_session.state IN (?, ?, ?, ?) + ) + ) + `), now, now, worktreeID, StatusDeleted, BranchOwnerManaged, expectedRecoveryHead, + models.TaskSessionStateCreated, models.TaskSessionStateStarting, + models.TaskSessionStateRunning, models.TaskSessionStateWaitingForInput) + if err != nil { + return false, err + } + rows, err := result.RowsAffected() + return rows == 1, err +} + +func (s *SQLiteStore) TouchArchivedBranchCandidate(ctx context.Context, worktreeID string) error { + _, err := s.db.ExecContext(ctx, s.db.Rebind(` + UPDATE task_environment_repos + SET updated_at = ? + WHERE worktree_id = ? + `), time.Now().UTC(), worktreeID) + return err +} + // scanWorktrees is a helper to scan multiple worktree rows. Task-keyed and // repository-keyed queries LEFT JOIN sessions (a worktree belongs to the task // environment, and a task may have zero or many sessions), so rows are @@ -594,7 +833,7 @@ func (s *SQLiteStore) scanWorktrees(rows *sql.Rows) ([]*Worktree, error) { seen := make(map[string]bool) for rows.Next() { wt := &Worktree{} - var mergedAt, deletedAt sql.NullTime + var compactedAt, mergedAt, deletedAt sql.NullTime var repositoryPath, baseBranch sql.NullString err := rows.Scan( @@ -608,6 +847,10 @@ func (s *SQLiteStore) scanWorktrees(rows *sql.Rows) ([]*Worktree, error) { &wt.Branch, &wt.BranchSlug, &baseBranch, + &wt.BranchOwner, + &wt.IntegrationRef, + &wt.RecoveryHeadSHA, + &compactedAt, &wt.Status, &wt.CreatedAt, &wt.UpdatedAt, @@ -628,6 +871,10 @@ func (s *SQLiteStore) scanWorktrees(rows *sql.Rows) ([]*Worktree, error) { if mergedAt.Valid { wt.MergedAt = &mergedAt.Time } + if compactedAt.Valid { + t := compactedAt.Time + wt.BranchCompactedAt = &t + } if deletedAt.Valid { wt.DeletedAt = &deletedAt.Time } @@ -688,6 +935,8 @@ func (s *SQLiteStore) GetWorktreeBySessionAndRepository(ctx context.Context, ses // Ensure SQLiteStore implements both Store and MultiRepoStore. var ( - _ Store = (*SQLiteStore)(nil) - _ MultiRepoStore = (*SQLiteStore)(nil) + _ Store = (*SQLiteStore)(nil) + _ MultiRepoStore = (*SQLiteStore)(nil) + _ BranchMetadataStore = (*SQLiteStore)(nil) + _ ArchivedBranchMaintenanceStore = (*SQLiteStore)(nil) ) diff --git a/apps/backend/internal/worktree/store_test.go b/apps/backend/internal/worktree/store_test.go index 92efb453ae..c194043bba 100644 --- a/apps/backend/internal/worktree/store_test.go +++ b/apps/backend/internal/worktree/store_test.go @@ -309,6 +309,64 @@ func TestSQLiteStore_ListActiveWorktreePaths(t *testing.T) { } } +func TestSQLiteStore_CountWorktreeBranchOwnersCountsPhysicalRepositoryClaimants(t *testing.T) { + store := newTestStore(t) + ctx := context.Background() + now := time.Now().UTC() + repoPath := filepath.Join(t.TempDir(), "repo") + + if _, err := store.db.ExecContext(ctx, ` + INSERT INTO workspaces (id, name, created_at, updated_at) + VALUES ('workspace', 'workspace', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP) + `); err != nil { + t.Fatalf("seed workspace: %v", err) + } + for _, repoID := range []string{"repo-owner-a", "repo-owner-b"} { + if _, err := store.db.ExecContext(ctx, ` + INSERT INTO repositories ( + id, workspace_id, name, local_path, created_at, updated_at + ) VALUES (?, 'workspace', ?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP) + `, repoID, repoID, repoPath); err != nil { + t.Fatalf("seed repository %s: %v", repoID, err) + } + } + for _, seed := range []struct { + sessionID string + taskID string + repoID string + wtID string + }{ + {"session-claimant-a", "task-claimant-a", "repo-owner-a", "wt-claimant-a"}, + {"session-claimant-b", "task-claimant-b", "repo-owner-b", "wt-claimant-b"}, + } { + store.seedSessionWithEnvironment(t, seed.sessionID, seed.taskID) + if err := store.CreateWorktree(ctx, &Worktree{ + ID: seed.wtID, + SessionID: seed.sessionID, + RepositoryID: seed.repoID, + RepositoryPath: repoPath, + Path: filepath.Join(t.TempDir(), seed.wtID), + Branch: "feature/shared", + BranchOwner: BranchOwnerManaged, + IntegrationRef: "main", + Status: StatusDeleted, + CreatedAt: now, + UpdatedAt: now, + DeletedAt: &now, + }); err != nil { + t.Fatalf("create %s: %v", seed.wtID, err) + } + } + + got, err := store.CountWorktreeBranchOwners(ctx, repoPath, "feature/shared") + if err != nil { + t.Fatalf("count branch owners: %v", err) + } + if got != 2 { + t.Fatalf("branch owners = %d, want 2", got) + } +} + func TestSQLiteStore_IgnoresInventoryOnlyRows(t *testing.T) { store := newTestStore(t) ctx := context.Background() diff --git a/apps/backend/internal/worktree/worktree.go b/apps/backend/internal/worktree/worktree.go index 9beb117eed..56695205e2 100644 --- a/apps/backend/internal/worktree/worktree.go +++ b/apps/backend/internal/worktree/worktree.go @@ -83,9 +83,24 @@ type Worktree struct { // internal provenance, so it is rebuilt when a snapshot is loaded. CleanupHeadOIDUnavailable bool `json:"-"` + // BranchCompactedAt records that exact-SHA local-ref deletion completed. + // A nil value keeps an interrupted archived candidate eligible for retry. + BranchCompactedAt *time.Time `json:"-"` // BaseBranch is the branch this worktree was created from. BaseBranch string `json:"base_branch"` + // BranchOwner identifies whether Kandev created the local branch ref. Only + // refs with BranchOwnerManaged are candidates for terminal compaction. + BranchOwner string `json:"-"` + + // IntegrationRef is the exact intended base/integration branch captured at + // materialization time. Cleanup never guesses this value from branch names. + IntegrationRef string `json:"-"` + + // RecoveryHeadSHA is the exact commit used to recreate a safely compacted + // managed branch after archive/unarchive. + RecoveryHeadSHA string `json:"-"` + // Status indicates the current state of the worktree. // Valid values: active, merged, deleted Status string `json:"status"` @@ -195,6 +210,10 @@ type CreateRequest struct { // same restart-safe recovery record identity. RecoveryOperationID string + // IntegrationRef is the verified branch against which terminal cleanup may + // prove a managed branch fully integrated. Empty fails closed. + IntegrationRef string + // FallbackBaseBranch is an optional branch to retry with when BaseBranch // does not exist in the repository. Typically populated with the // repository's default_branch by the caller. When empty, a missing diff --git a/docs/decisions/2026-08-30-compact-integrated-managed-branches.md b/docs/decisions/2026-08-30-compact-integrated-managed-branches.md new file mode 100644 index 0000000000..d7338c5ecb --- /dev/null +++ b/docs/decisions/2026-08-30-compact-integrated-managed-branches.md @@ -0,0 +1,112 @@ +# ADR-2026-08-30-compact-integrated-managed-branches: Compact Fully Integrated Managed Branches + +**Status:** accepted +**Date:** 2026-08-30 +**Area:** backend, operations + +## Context + +Task lifecycle cleanup deliberately removed Git worktree registrations while +retaining every local branch ref. That protected unpublished commits and made +archive recovery possible, but no bounded automatic consumer reclaimed managed +branches after their commits were fully integrated. The result was permanent +growth of redundant local refs. + +A branch name or task-shaped suffix is not proof of ownership. A branch can be +user-selected, shared with another environment, checked out in another Git +worktree, or contain commits absent from the intended base. Archive recovery also +cannot depend on a remote branch: unpublished state must restore exactly, and a +safely removed integrated branch must remain locally recoverable. + +## Decision + +Capture four internal fields on each `task_environment_repos` worktree row: + +- branch owner: `kandev`, `external`, or `unknown`; +- intended integration ref, resolved from the task branch policy target before + falling back to the task repository base branch; +- exact recovery head SHA, initially empty. +- branch-compaction completion timestamp, initially empty. + +Legacy and incomplete rows default to `unknown` and empty refs. They are retained +without inference. + +At terminal worktree cleanup, after removing the worktree registration and while +holding the existing repository lock, consider exactly one explicit local branch. +Delete it only when all of these conditions hold: + +1. The persisted owner is `kandev` and exactly one durable row claims the + repository/branch pair. +2. No live Git worktree registration uses `refs/heads/`. +3. The branch is not its base or integration ref and both exact commit refs + resolve locally without fetching. +4. `git merge-base --is-ancestor ` succeeds. +5. The exact branch head is persisted with compare-and-set, then an opaque + worktree-identity-derived local recovery ref is created at that head before + deletion. The recovery ref keeps the commit reachable if the integration ref + is rewritten or Git prunes unreachable objects. +6. `git update-ref -d refs/heads/ ` atomically removes + the single explicit local ref only if its head is unchanged. +7. A post-delete liveness probe restores the exact ref with zero-OID + compare-and-set if a Git worktree appeared during deletion; only a completed + deletion records the separate completion timestamp. + +Any missing metadata, ambiguous owner, active/shared reference, protected ref, +failed probe, unique commit, compare-and-set loss, head race, or refused +non-force deletion retains the branch. Remote refs are never deletion targets. +Permanent task deletion keeps its separate explicit force-delete disposition. + +Unarchive and recreate use a retained local branch directly. When safe +compaction removed the local ref, they recreate it from the persisted exact head +before attempting remote or pull-request recovery. Any existing same-named +local ref must resolve to that exact head; a mismatch fails closed. Once the +exact local branch exists, recovery clears the compaction marker and removes the +opaque recovery ref, allowing a later archive without a session launch to be +considered again. + +All terminal paths share the manager policy: single-repository cleanup, +multi-repository cleanup, task archive, task-environment reset, handoff cleanup, +and aged automation cleanup. The task cleanup service delegates worktree removal +to its batch cleaner exactly once. Aggregate attempted/deleted/retained receipts, +fixed retained-reason counts, and matching expvar counters provide bounded +observability without branch lists or repository data. + +Archive cleanup is the first attempt, not the only attempt. A storage-maintenance +provider selects at most 100 Kandev-owned rows whose tasks remain archived and +whose worktrees are inactive and deleted. It delegates every selected row to the +same manager policy and contains no branch-name scan or deletion logic. The +manager conditionally persists the recovery head while the task is archived, +rechecks archived state immediately before ref mutation, and rotates retained +candidates by durable update time so bounded runs make progress. A safely +compacted row keeps its recovery head, records its completion timestamp, and is +no longer selected. A crash after recovery persistence but before deletion +leaves the timestamp empty, so a later bounded pass revalidates and retries it. +Rematerializing the worktree clears the completion timestamp. + +## Consequences + +- Fully integrated Kandev-created local branches no longer accumulate without + bound after terminal cleanup. +- Unpublished, external, legacy, protected, shared, inherited, and ambiguous + branches remain fail-closed. +- Archive/unarchive preserves exact state whether the branch ref was retained or + safely compacted. +- Branches archived before integration receive a bounded later retry without an + install-wide Git ref scan. +- New additive columns must survive SQLite and PostgreSQL ownership migrations + and environment-repository CRUD. +- Expected-SHA deletion conservatively retains a branch advanced after ancestry + proof. Safety takes precedence over reclamation rate. + +## Alternatives Considered + +1. **Set every cleanup call to `removeBranch=true`.** Rejected because the + existing force deletion discards unpublished work and makes some archived + branches unrecoverable. +2. **Delete branches matching the generated naming pattern.** Rejected because + names do not prove ownership, ancestry, liveness, or task-environment identity. +3. **Run an install-wide periodic branch glob collector.** Rejected because it + loses the lifecycle metadata and repository lock already available at terminal + cleanup, broadens the deletion surface, and creates duplicate attempts. +4. **Keep every local branch indefinitely.** Rejected because redundant managed + refs accumulate permanently after their commits are integrated. diff --git a/docs/decisions/2026-09-07-separate-runtime-permissions-and-advisory-actions.md b/docs/decisions/2026-09-07-separate-runtime-permissions-and-advisory-actions.md index a474004c62..4979241f54 100644 --- a/docs/decisions/2026-09-07-separate-runtime-permissions-and-advisory-actions.md +++ b/docs/decisions/2026-09-07-separate-runtime-permissions-and-advisory-actions.md @@ -1,7 +1,7 @@ # ADR-2026-09-07-separate-runtime-permissions-and-advisory-actions: Keep live advisory actions separate from runtime permissions -**Status:** accepted -**Date:** 2026-09-07 +**Status:** accepted +**Date:** 2026-09-07 **Area:** backend, protocol ## Context diff --git a/docs/plans/compact-integrated-managed-branches/plan.md b/docs/plans/compact-integrated-managed-branches/plan.md new file mode 100644 index 0000000000..310945b184 --- /dev/null +++ b/docs/plans/compact-integrated-managed-branches/plan.md @@ -0,0 +1,98 @@ +--- +created: 2026-09-14 +status: implemented +requirements: + - REQ-TASKS-MANAGED-BRANCH-COMPACTION-001 +system_design: + - ../../specs/tasks/system-design/managed-branch-compaction.md +legacy_specs: [] +--- + +# Implementation Plan: Compact Integrated Managed Branches + +## Overview + +Bound generated local branch growth at task cleanup boundaries while retaining +every branch that cannot be proven safe to remove. Persist ownership, +integration, and exact recovery metadata first; centralize eligibility and +expected-head deletion in the worktree manager; then reuse that policy from +terminal cleanup and bounded archived maintenance. + +## Scope + +### In scope + +- Persist conservative managed-branch ownership, integration, recovery-head, + and completion metadata on canonical task-environment repository rows. +- Compact one exact local branch only after ownership, liveness, protected-ref, + and integration-ancestry checks succeed under the repository lock. +- Restore an exact compacted head during unarchive/recreation and keep recovery + interruption ordering retryable. +- Route task cleanup, handoff cleanup, environment reset, aged automation + cleanup, and archived storage maintenance through the shared policy. +- Emit bounded receipts, logs, and cumulative metrics. + +### Out of scope + +- Branch-name, age, or glob-based ownership inference. +- Network fetches, remote-ref deletion, or force deletion in preserving cleanup. +- Changing explicit permanent task-deletion semantics or adding UI. + +## Technical approach + +Extend `TaskEnvironmentRepo` and the normalized schema with conservative branch +metadata and compare-and-set repository operations. Propagate the verified +integration ref from branch policy through environment preparation and executor +persistence. After worktree registration removal, `worktree.Manager` evaluates +one deduplicated candidate under its repository lock, creates an opaque recovery +ref, persists the exact head, and removes only +`refs/heads/` with the expected SHA. + +Archive retains ineligible branches. Storage maintenance selects at most 100 +archived, inactive, deleted managed rows, rechecks archived state, and delegates +to the same manager. Recovery recreates the exact local ref before remote +materialization, removes the recovery ref before clearing its metadata, and +supports idempotent retry across interruptions. + +## Tests + +- AC-TASKS-MANAGED-BRANCH-COMPACTION-001.1: cleanup policy tests cover managed ownership, + ambiguous/legacy/external/shared rows, liveness, protected refs, integration, + and concurrent cleanup. +- AC-TASKS-MANAGED-BRANCH-COMPACTION-001.2: cleanup, archived maintenance, GC reachability, + recreate, and `RecoverBranchStatus` tests prove exact restoration and + interruption-safe recovery finalization. +- AC-TASKS-MANAGED-BRANCH-COMPACTION-001.3: command-boundary tests prove one explicit + local expected-head deletion and retention on head races. +- AC-TASKS-MANAGED-BRANCH-COMPACTION-001.4: receipt and metrics tests cover fixed reason + labels and deduplicated counts. +- AC-TASKS-MANAGED-BRANCH-COMPACTION-001.5: archived maintenance tests cover bounded + selection, archive/session races, later integration, idempotency, and live-ref + restoration. + +## Work orders + +- [x] [Task 01: Implement safe managed-branch compaction and recovery](task-01-managed-branch-compaction.md) + +## Verification results + +- RED: `TestRecoverBranchStatus_FinalizesInterruptedPreDeleteRecovery` and + `TestRecoverBranchStatus_RetainsMetadataWhenRecoveryRefDeleteFails` failed on + head `986e745fc0b714cf383d9c86e78a99d8926d4c82` before the recovery-ordering fix. +- The complete `internal/worktree` suite and the focused race-enabled cleanup, + maintenance, recovery, and recreate suite passed after the fix. +- Focused SQLite repository, backend application, and task service suites passed. +- SQL guard, differential Go lint against `origin/main`, backend build, spec + catalog validation, spec lint, delivery-package coverage, and diff checks + passed. +- Full `make -C apps/backend lint` reaches 35 violations in current + `origin/main` files outside this change; differential lint reports zero issues. + +## Risks + +- A stale ownership or integration record could authorize data loss; every + incomplete or inconsistent input must retain the branch. +- Git liveness and branch heads can change outside the manager lock; expected-SHA + ref updates and post-delete liveness repair must remain fail closed. +- Recovery metadata and its opaque ref can diverge across interruption; ordering + must leave either step safely retryable. diff --git a/docs/plans/compact-integrated-managed-branches/task-01-managed-branch-compaction.md b/docs/plans/compact-integrated-managed-branches/task-01-managed-branch-compaction.md new file mode 100644 index 0000000000..bcc36a1236 --- /dev/null +++ b/docs/plans/compact-integrated-managed-branches/task-01-managed-branch-compaction.md @@ -0,0 +1,118 @@ +--- +id: "01-managed-branch-compaction" +title: "Implement safe managed-branch compaction and recovery" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-TASKS-MANAGED-BRANCH-COMPACTION-001 +acceptance_criteria: + - AC-TASKS-MANAGED-BRANCH-COMPACTION-001.1 + - AC-TASKS-MANAGED-BRANCH-COMPACTION-001.2 + - AC-TASKS-MANAGED-BRANCH-COMPACTION-001.3 + - AC-TASKS-MANAGED-BRANCH-COMPACTION-001.4 + - AC-TASKS-MANAGED-BRANCH-COMPACTION-001.5 +system_design: + - ../../specs/tasks/system-design/managed-branch-compaction.md +--- + +# Task 01: Implement Safe Managed-Branch Compaction and Recovery + +## Summary + +Implement the complete backend slice that records authoritative branch +metadata, safely compacts fully integrated managed local branches, restores +their exact heads, and revisits retained archived branches through bounded +storage maintenance. + +## In scope + +- Canonical task-environment repository schema, migrations, CRUD, and CAS. +- Integration-ref propagation through lifecycle and executor materialization. +- Central manager eligibility, exact local-ref deletion, recovery refs, + receipts, metrics, and concurrency behavior. +- All task cleanup callers, handoff cleanup, and archived maintenance wiring. +- Exact-head unarchive/recreate recovery and interruption remediation. +- Durable contracts, ADR, public operations guidance, and focused tests. + +## Out of scope + +- UI, browser E2E, remote mutation, network fetch during cleanup, and historical + ownership backfill. + +## Acceptance + +- Only a uniquely owned, inactive, unprotected managed local branch whose exact + head is contained by its persisted integration ref can be compacted. +- Compaction and recovery preserve an exact reachable head across concurrent or + interrupted operations, and every ambiguity retains the branch. +- Every named terminal caller and bounded archived maintenance uses the shared + policy and exposes bounded observability. + +## Verification + +```bash +(cd apps/backend && go test ./internal/worktree -count=1) +(cd apps/backend && go test -race ./internal/worktree -run 'MaintainArchivedBranches|BranchCleanup|Concurrent|CleanupWorktreesPreservingBranches|RecoverBranchStatus|Recreate_RecoveryHead' -count=1) +(cd apps/backend && go test ./internal/task/repository/sqlite -run 'Cutover|ArchivedBranch|Worktree|RecoveryHead|Compaction' -count=1) +(cd apps/backend && go test ./internal/backendapp -run 'StorageMaintenance|ArchivedBranch|Worktree' -count=1) +(cd apps/backend && go test ./internal/task/service -run 'BranchRecovery|TaskEnvironment|Cleanup|Handoff' -count=1) +(cd apps/backend && go run ./cmd/sqlguard ./internal) +(cd apps/backend && golangci-lint run --new-from-rev=origin/main ./...) +make -C apps/backend build +python3 scripts/list-docs.py validate +python3 scripts/lint-spec-files.py --all +git diff --check origin/main...HEAD +``` + +## Files likely touched + +- `apps/backend/internal/agent/runtime/lifecycle/` +- `apps/backend/internal/backendapp/` +- `apps/backend/internal/orchestrator/executor/` +- `apps/backend/internal/task/models/models.go` +- `apps/backend/internal/task/repository/sqlite/` +- `apps/backend/internal/task/service/` +- `apps/backend/internal/worktree/` +- `docs/decisions/2026-08-30-compact-integrated-managed-branches.md` +- `docs/public/` +- `docs/specs/tasks/requirements/managed-branch-compaction.md` +- `docs/specs/tasks/system-design/managed-branch-compaction.md` + +## Dependencies + +None. + +## Risks + +- A mistake in ownership, ancestry, liveness, or recovery ordering can delete or + strand local-only work; all gates and mutations must fail closed. + +## Parallelism + +`sequential` + +## Inputs + +- REQ-TASKS-MANAGED-BRANCH-COMPACTION-001 and AC-TASKS-MANAGED-BRANCH-COMPACTION-001.1 through + AC-TASKS-MANAGED-BRANCH-COMPACTION-001.5. +- Task runtime-cleanup system design and the managed-branch compaction ADR. +- Existing worktree manager locking, task cleanup, and storage-maintenance + patterns. + +## Results + +- Added conservative ownership and integration metadata, exact recovery refs, + expected-head local deletion, bounded receipts/metrics, and archived storage + maintenance through the shared worktree-manager policy. +- Wired every named terminal cleanup path and exact-head unarchive/recreate + recovery while retaining legacy, external, shared, protected, live, + ambiguous, and unpublished branches. +- Corrected recovery finalization so an interrupted pre-delete recovery clears + after confirming the exact local head, and so the recovery ref is removed + before its database metadata. Both interruption regressions failed before the + fix and pass afterward. +- All verification commands in this work order passed. Current-main full lint + has 35 baseline findings outside the PR diff; differential lint passed with + zero issues. diff --git a/docs/public/executors.md b/docs/public/executors.md index 07e644d774..94ad2d63dc 100644 --- a/docs/public/executors.md +++ b/docs/public/executors.md @@ -160,6 +160,13 @@ exactly match the repository. A broker-aware `gh` shim redeems the primary repos each invocation, sets `GH_TOKEN` only on the child `gh` process, and isolates CLI configuration from the host. +For Local and Worktree tasks, managed credentials cover Kandev's checkout operations and the +launched task processes. Per-repository setup scripts still receive executor-profile and repository +environment bindings, user-configured Git settings, and Kandev's managed build cache, but Kandev +removes broker leases and generated Git and `gh` helper routing before those scripts start. If a +repository setup script needs authenticated GitHub access, configure an explicit scoped profile or +repository credential, or select **Inherit executor Git credentials** and configure the host. + When the workspace uses a GitHub App, the redeemed installation token is minted for that one repository. On a multi-repository task, Git can redeem each repository's lease, but App-backed `gh` commands are primary-repository scoped. Run cross-repository GitHub API work through Kandev's diff --git a/docs/public/git-operations.md b/docs/public/git-operations.md index 631c23be11..6d089b2d8a 100644 --- a/docs/public/git-operations.md +++ b/docs/public/git-operations.md @@ -377,8 +377,10 @@ Read-only Git actions used by the Changes panel include `session.commit_diff`, ` Worktree cleanup audits the Git worktree and checkout before it runs the repository cleanup script. It then removes the Git worktree directory and may remove the local branch: - Normal task deletion audits each owned worktree before mutation. Tracked or untracked changes stop deletion and keep the task, checkout, and local branch in place until the user explicitly consents to discard them. With consent, cleanup removes the checkout only after the normal ownership and identity audits pass. A clean branch is removed only when its current commit is already contained by the recorded base or repository default; a clean branch with unique commits is preserved after its checkout is reclaimed. Remote branches are never deleted. -- **Reset Environment** is allowed only when no task session is `STARTING` or `RUNNING`. It can optionally push first; a failed requested push aborts the reset. Teardown removes the worktree but deliberately preserves the local branch, then the next launch materializes a fresh environment. -- Office handoff cleanup also preserves the branch when it releases a worktree. +- **Reset Environment** is allowed only when no task session is `STARTING` or `RUNNING`. It can optionally push first; a failed requested push aborts the reset. Teardown removes the worktree, preserves unpublished or ambiguous local work, and removes a Kandev-owned local branch only when its exact head is already contained in the recorded integration ref. The next launch materializes a fresh environment. +- Office handoff cleanup uses the same fail-closed local-branch policy when it releases a worktree. + +Archive cleanup uses that policy too. If a task is archived before integration, the branch remains. A later full storage-maintenance run can revisit a bounded set of those archived rows and remove the exact local ref after integration is proven. Ref deletion includes the expected head SHA, so an external branch update causes retention instead of deleting the changed ref. Kandev never removes a remote ref through this cleanup. Before deleting a task or performing a hard reset, commit and push anything you need. Without discard consent, Kandev does not start a cleanup job when the audit finds uncommitted or untracked work. With consent, cleanup scripts run only after the normal worktree audit and perform transient teardown; files they create are removed with the audited checkout. If an audited cleanup script fails, Kandev logs the failure and continues with the same recorded worktree. An audited directory is removed through its pinned no-follow handle, and un-audited fallback cleanup can remove a managed directory without following replacement links. Git metadata is then pruned. Registration pruning and local-branch deletion are verified before the durable cleanup job succeeds; a partial failure remains retryable. @@ -396,6 +398,6 @@ Before deleting a task or performing a hard reset, commit and push anything you - **Change-request creation failed after push:** the branch may already be remote. Fix `gh`, `glab`, GitLab workspace-token, or `az` authentication as applicable, then retry without assuming the push was rolled back. GitLab retries reuse an existing open MR with the same source and target. - **Operation timed out:** inspect status before retrying. A client timeout or lost WebSocket response does not prove the underlying command did nothing. - **Multi-repository operation failed at workspace root:** choose the repository in the toolbar or include its exact `repo` subpath in the request. -- **Missing work after cleanup:** inspect the preserved local branch for Reset Environment/handoff, or the remote branch if it was pushed. Task deletion may already have force-deleted the local branch. +- **Missing work after cleanup:** inspect a retained local branch or the remote branch if it was pushed. On unarchive, Kandev recreates a safely compacted managed branch from its recorded exact head before trying remote recovery. Task deletion may already have force-deleted the local branch. Related guides: [Configuration](configuration.md), [Executors](executors.md), [Operations](operations.md), and [WebSocket API](websocket-api.md). diff --git a/docs/public/operations.md b/docs/public/operations.md index d2ea1b042e..06690f158d 100644 --- a/docs/public/operations.md +++ b/docs/public/operations.md @@ -260,6 +260,13 @@ permanent deletion. Each entry shows its `delete_after` retention deadline: **De time, not an exact promise, the first successful scheduled or full manual maintenance run after the deadline performs the purge, subject to the idle gate and any preemption. +Each full maintenance run also revisits at most 100 Kandev-managed local branches retained when a +task was archived before its work was integrated. Kandev considers only durable rows that still +belong to archived tasks with inactive worktrees; it does not scan branch-name patterns or fetch +remote state. The branch is removed only after the same ownership, liveness, and local integration +checks used during archive cleanup. Remote refs and branches with unpublished or ambiguous work +remain untouched. + ![Settings > System > Storage showing the maintenance policy, schedule, workspace cleanup, and folder allowlist.](../screenshots/system-maintenance-policy.png) Use **Clear eligible** to remove only entries whose deadlines have passed. It reports protected diff --git a/docs/public/tasks-and-workflows.md b/docs/public/tasks-and-workflows.md index 5f8dcdc05b..b653e7d11b 100644 --- a/docs/public/tasks-and-workflows.md +++ b/docs/public/tasks-and-workflows.md @@ -772,7 +772,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 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. | +| Git worktree | Attempts to remove the Kandev-owned worktree directory. It removes a managed local branch only when its exact head is already integrated; unpublished, external, ambiguous, shared, or borrowed work remains. Remote branches are untouched. | | Local Docker | Attempts to stop and remove the container; the host repository remains. | | Kubernetes | Deletes only the recorded Pod and Kandev-managed PVC after exact UID and ownership checks. An existing claim is retained. | | 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. | @@ -787,7 +787,7 @@ While a task is archived, Kandev shows its history but does not start its agent If session startup or resume fails, Kandev keeps the failure as a chronological entry in that session's chat. The current unresolved entry provides the valid recovery actions. After recovery and later agent output, the entry remains in the chat with its details, while older entries have no stale controls. History loading and new messages keep the normal chat scroll position. -If task or workspace preparation fails, Kandev shows one task error strip below the task header and above the session and Plan tabs. The strip stays visible when you switch sessions or tabs. Select **Show details** to open the available guarded actions in a desktop dialog or phone drawer. Kandev clears the strip after task recovery succeeds. If you archive the task during recovery, Kandev stops that recovery path and does not start a fallback restore. 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. +If task or workspace preparation fails, Kandev shows one task error strip below the task header and above the session and Plan tabs. The strip stays visible when you switch sessions or tabs. Select **Show details** to open the available guarded actions in a desktop dialog or phone drawer. Kandev clears the strip after task recovery succeeds. If you archive the task during recovery, Kandev stops that recovery path and does not start a fallback restore. For worktree tasks, archive keeps the environment identity and either retains the local branch or records its exact integrated head before safe compaction. The next session recreates the worktree directory and restores a missing managed branch from that exact head before trying `origin`. Recovery is best-effort and does not rewrite ambiguous multi-row attachments for the same repository. If neither exact local recovery nor a remote branch is available, 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 retry and restart-reconciliation rules as archive. diff --git a/docs/specs/integrations/system-design/github-authentication-01.md b/docs/specs/integrations/system-design/github-authentication-01.md index 947d6fce16..6cbe8033e8 100644 --- a/docs/specs/integrations/system-design/github-authentication-01.md +++ b/docs/specs/integrations/system-design/github-authentication-01.md @@ -133,8 +133,11 @@ automation under different GitHub Apps without operating separate Kandev deploym - Managed Git helper execution does not depend on the post-startup `PATH`: Git resolves an absolute Kandev-owned `agentctl` executable published before the first managed Git operation. Local and Worktree preparation binds the helper to the standalone launcher's absolute executable - before checkout or setup scripts run. Remote preparation binds it to the installed executor - binary before cloning, and a running `agentctl` publishes its own executable for child processes. + before Kandev-owned checkout operations run. Per-repository setup scripts retain resolved profile + and repository environment, user-owned indexed Git configuration, and Kandev's managed build + cache, but Kandev removes broker capabilities and generated Git and `gh` helper routing before + those scripts start. Remote preparation binds the helper to the installed executor binary before + cloning, and a running `agentctl` publishes its own executable for child processes. Non-interactive Unix login shells that replace their inherited `PATH` restore the managed CLI-shim directory after profile initialization for broker-enabled tasks, while preserving pre-existing Bash environment hooks, including hook paths containing `$VAR` or `${VAR}` diff --git a/docs/specs/integrations/system-design/github-authentication-03.md b/docs/specs/integrations/system-design/github-authentication-03.md index a91e49972e..48dc7764a1 100644 --- a/docs/specs/integrations/system-design/github-authentication-03.md +++ b/docs/specs/integrations/system-design/github-authentication-03.md @@ -160,10 +160,14 @@ This design preserves the technical source detail for `REQ-INTEGRATIONS-GITHUB-A - **GIVEN** a broker-enabled managed task whose login profile replaces its inherited `PATH`, **WHEN** Git requests GitHub HTTPS credentials, **THEN** the configured helper invokes the instance-owned `agentctl` directly and does not search or fall through to an ambient helper. -- **GIVEN** a broker-enabled Local or Worktree task whose checkout or setup script invokes Git - before the task instance is created, **WHEN** Git requests GitHub HTTPS credentials, **THEN** the +- **GIVEN** a broker-enabled Local or Worktree task whose Kandev-owned checkout invokes Git before + the task instance is created, **WHEN** Git requests GitHub HTTPS credentials, **THEN** the configured helper invokes the standalone launcher's absolute `agentctl` executable without consulting `PATH` or an ambient helper. +- **GIVEN** a Local or Worktree per-repository setup script, **WHEN** Kandev builds its environment, + **THEN** the script receives resolved profile and repository values, user-owned indexed Git + configuration, and Kandev's managed build cache, but receives no managed broker capability, + Kandev-generated Git credential helper, or managed `gh` shim routing. - **GIVEN** a broker-enabled Docker or Sprites task whose prepare script clones before `agentctl` starts, **WHEN** Git requests GitHub HTTPS credentials during that clone, **THEN** the configured helper invokes the already-installed absolute executor binary and redeems the task lease without diff --git a/docs/specs/tasks/README.md b/docs/specs/tasks/README.md index 3102237fc7..b7bb439b74 100644 --- a/docs/specs/tasks/README.md +++ b/docs/specs/tasks/README.md @@ -89,6 +89,7 @@ signals, and task-scoped scheduling contracts. - [Queued run scheduling](requirements/run-scheduling.md) - [Resume prompt queue](requirements/resume-prompt-queue.md) - [Task Runtime Cleanup](requirements/runtime-cleanup.md) +- [Managed Branch Compaction](requirements/managed-branch-compaction.md) - [Task Terminal Persistence](requirements/task-terminal-persistence.md) - [Runtime Task-State Publication Order](requirements/runtime-state-publication-order.md) - [Session Delete Preserves Task Workspaces](requirements/session-delete-resource-cleanup.md) @@ -181,6 +182,7 @@ signals, and task-scoped scheduling contracts. - [Task plan append-mode write](system-design/plan-write-append-mode.md) - [Task plan append-mode agent text](system-design/plan-write-append-mode-agent-text.md) - [Task Runtime Cleanup](system-design/runtime-cleanup.md) +- [Managed Branch Compaction](system-design/managed-branch-compaction.md) - [Task Cleanup Preparation](system-design/runtime-cleanup-preparation.md) - [Task Terminal Persistence](system-design/task-terminal-persistence.md) - [Runtime Task-State Publication Order](system-design/runtime-state-publication-order.md) diff --git a/docs/specs/tasks/requirements/managed-branch-compaction.md b/docs/specs/tasks/requirements/managed-branch-compaction.md new file mode 100644 index 0000000000..308d5ad438 --- /dev/null +++ b/docs/specs/tasks/requirements/managed-branch-compaction.md @@ -0,0 +1,66 @@ +--- +status: draft +system: tasks +created: 2026-08-30 +updated: 2026-09-14 +owners: + - cfl +--- + +# Managed Branch Compaction Requirements + +## Overview + +Task lifecycle cleanup removes Git worktree registrations but must bound the +growth of local branches that Kandev itself created. Compaction may delete a +managed local branch only when ownership, liveness, and integration are proven +from persisted metadata; every unproven case retains the branch. Deleted +branches remain exactly recoverable from their recorded head. + +The decision record is +[ADR-2026-08-30-compact-integrated-managed-branches](../../../decisions/2026-08-30-compact-integrated-managed-branches.md). + +## Requirements + +### REQ-TASKS-MANAGED-BRANCH-COMPACTION-001: Managed Branch Compaction + +**Intent:** Bound generated local branch growth at terminal task boundaries +without inferring ownership from branch names, force-deleting work, touching +remote refs, or breaking archive and unarchive. + +#### Acceptance criteria + +- **AC-TASKS-MANAGED-BRANCH-COMPACTION-001.1:** When terminal lifecycle + cleanup removes a worktree registration, it shall delete a local branch only + when the persisted owner is Kandev, no live Git worktree uses the branch, + exactly one durable environment-repository row owns it, and its exact head + is contained in the persisted intended integration ref. Every other case + shall retain the branch. +- **AC-TASKS-MANAGED-BRANCH-COMPACTION-001.2:** Before deleting an eligible + integrated branch, cleanup shall persist its exact head SHA and create an + opaque, worktree-identity-derived local recovery ref at that SHA before + removing the branch. Unarchive and worktree recreation shall restore a + missing managed branch from that SHA before remote recovery, then clear the + compacted marker and remove the recovery ref only after the exact local + branch protects the commit. An existing local branch must equal the recorded + head or recovery fails closed. Branches with unpublished commits retain their + original local ref and restore exactly. +- **AC-TASKS-MANAGED-BRANCH-COMPACTION-001.3:** Managed branch compaction + shall delete only one explicit local ref with an atomic expected-head + compare-and-delete. It shall never delete remote refs, protected or base + refs, inferred branch globs, externally owned refs, or refs with legacy, + missing, or ambiguous ownership metadata. +- **AC-TASKS-MANAGED-BRANCH-COMPACTION-001.4:** Terminal cleanup shall emit + bounded attempted, deleted, and retained totals plus fixed retained-reason + counts. Receipts and metrics shall not contain branch lists, repository + contents, or credentials. +- **AC-TASKS-MANAGED-BRANCH-COMPACTION-001.5:** When archive cleanup retains a + managed branch because it is not yet integrated, storage maintenance shall + revisit at most a fixed number of archived, inactive worktree rows per run. + It shall revalidate that the task remains archived immediately before + invoking the worktree manager's existing safety policy. A later unarchive + shall restore a safely compacted branch from its exact recorded head. + Persisting that head without completing deletion shall remain retryable + after interruption, while a durable completion marker shall prevent completed + rows from being selected again. A branch that becomes live during deletion + shall be restored at the exact recorded head and retained. diff --git a/docs/specs/tasks/system-design/managed-branch-compaction.md b/docs/specs/tasks/system-design/managed-branch-compaction.md new file mode 100644 index 0000000000..eb51435298 --- /dev/null +++ b/docs/specs/tasks/system-design/managed-branch-compaction.md @@ -0,0 +1,94 @@ +--- +status: draft +system: tasks +requirements: + - REQ-TASKS-MANAGED-BRANCH-COMPACTION-001 +created: 2026-08-30 +updated: 2026-09-14 +owners: + - cfl +--- + +# Managed Branch Compaction System Design + +## Purpose and boundaries + +This design extends +[Task Runtime Cleanup](runtime-cleanup.md) for the disposition of managed local +branches at terminal task boundaries. It defines capture of ownership and +integration metadata on canonical task-environment repository rows, the +fail-closed eligibility checks under the repository lock, the exact-head +recovery protocol, bounded archived-branch maintenance, and the bounded +cleanup receipt. + +It does not change task deletion semantics, remote refs, branch-policy +configuration, or archive and unarchive state ownership. + +## Requirement mapping + +| Requirement | Design source | +| --- | --- | +| REQ-TASKS-MANAGED-BRANCH-COMPACTION-001 | Sections below | + +## Decision source + +[ADR-2026-08-30-compact-integrated-managed-branches](../../../decisions/2026-08-30-compact-integrated-managed-branches.md) + +## Metadata capture + +Every task-environment repository row that materializes a worktree branch +persists three conservative fields, each defaulting to unknown or empty for +legacy rows without inference: + +- `worktree_branch_owner`: `kandev`, `external`, or `unknown`; +- `worktree_integration_ref`: the intended base or integration branch, resolved + from the task branch policy pull-request target first, then the contribution + destination target, then the task repository base branch; +- `worktree_recovery_head_sha` plus `worktree_branch_compacted_at`: the exact + compaction recovery state, empty until a deletion is prepared. + +Missing or ambiguous metadata always retains the branch. + +## Terminal compaction boundary + +Immediately after a terminal cleanup successfully removes the worktree +registration, while still holding the existing repository lock, cleanup +considers exactly one explicit candidate local branch. The candidate is +eligible only when the persisted owner is `kandev`, exactly one durable +environment-repository row owns the repository and branch identity, no live Git +worktree uses the branch, the branch is neither protected nor the repository +default, base, or integration ref, and the branch's exact tip is an ancestor of +the exact persisted integration ref. Cleanup never fetches, globs, enumerates +unrelated refs, or infers eligibility from names, age, or convention. + +Before deletion, cleanup persists the exact candidate head SHA with +compare-and-set semantics, then removes the branch with +`git update-ref -d refs/heads/ `. An expected-head +mismatch or any check failure retains the branch and leaves the state retryable. + +## Exact-head recovery + +Recreation after archive or unarchive restores a missing managed branch from +the recorded exact head before any refreshed-origin or contribution +materialization can select a different head. A local branch of the same name +must resolve to the recorded head or recovery fails closed. The hidden +identity-derived recovery ref is deleted before the durable recovery fields are +CAS-cleared, so every interruption point remains retryable. Branches that were +never compacted keep the existing preserved-local-ref behavior and restore +without rewriting their tip. + +## Bounded archived maintenance + +Storage maintenance selects at most 100 archived, inactive, deleted-worktree +rows per run without scanning names, revalidates archived state and session +inactivity immediately before mutation, and reuses the same manager-owned +policy. Interrupted rows retry; a durable completion marker prevents +reselection. A branch that becomes live during deletion is restored at the +exact recorded head and retained. + +## Observability + +Each cleanup operation returns a bounded receipt with attempted, deleted, and +retained counts plus fixed retained-reason counts. Logs and metrics expose the +same bounded labels without branch lists, secrets, commit contents, or +unbounded cardinality. diff --git a/docs/specs/tasks/system-design/runtime-cleanup.md b/docs/specs/tasks/system-design/runtime-cleanup.md index a1ed8bab1e..a3cd47f368 100644 --- a/docs/specs/tasks/system-design/runtime-cleanup.md +++ b/docs/specs/tasks/system-design/runtime-cleanup.md @@ -12,7 +12,7 @@ owners: ## Purpose and boundaries -This design defines task runtime cleanup for REQ-TASKS-RUNTIME-CLEANUP-001. +This design owns the technical contract for REQ-TASKS-RUNTIME-CLEANUP-001. The active implementation authority is split into [Archive Cleanup Evidence](archive-cleanup-evidence.md), @@ -31,6 +31,9 @@ own exact evidence, startup symbols, and table names. Decision: [ADR-2026-08-08-task-owned-worktree-lifetime](../../../decisions/2026-08-08-task-owned-worktree-lifetime.md) +Branch retention amendment: +[ADR-2026-08-30-compact-integrated-managed-branches](../../../decisions/2026-08-30-compact-integrated-managed-branches.md) + ## Why Operators archive and delete completed tasks to keep the board usable and the @@ -132,6 +135,9 @@ by an earlier pass. Unarchive reuses that environment and branch; absent live repository rows use normal preparation. Delete may remove owner rows only after capturing the cleanup snapshot. +Managed branch compaction amends that disposition in +[Managed Branch Compaction](managed-branch-compaction.md). + ## Data Model Preparation with absent resources follows @@ -389,8 +395,8 @@ reported missing rather than exposed. 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. +- Archive cleanup preserves the task environment owner and exact branch recovery + state. Cleanup retries 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. @@ -521,8 +527,7 @@ reported missing rather than exposed. 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. + remote branch is absent, **THEN** exact local recovery state remains available. - **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 @@ -546,3 +551,4 @@ reported missing rather than exposed. - [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) +- [Compact integrated managed branches](../../../plans/compact-integrated-managed-branches/plan.md)