diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go b/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go index 7b8bc8cb64..460de6f73e 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go @@ -1701,10 +1701,11 @@ func validateLaunchWorkspaceAdmission(ctx context.Context, req *LaunchRequest, w } for index, repository := range repositories { candidate := workspacePath + entry := workspaceRepositoryEntryName(repository.RepoName) if index > 0 { - candidate = filepath.Join(workspacePath, repository.RepoName) + candidate = filepath.Join(workspacePath, entry) } else if len(repositories) > 1 && validateLocalRepositoryWorkspace(ctx, candidate, repository.RepositoryPath) != nil { - candidate = filepath.Join(workspacePath, repository.RepoName) + candidate = filepath.Join(workspacePath, entry) } // A missing worktree during ACP resume must reach WorktreePreparer. // It classifies a deleted branch and returns the typed recovery error @@ -1720,6 +1721,19 @@ func validateLaunchWorkspaceAdmission(ctx context.Context, req *LaunchRequest, w return nil } +// workspaceRepositoryEntryName returns the directory segment below the task +// root that holds a repository's checkout. A launch spec carries the +// repository's display name, which may contain path separators or a drive +// letter; the worktree manager sanitizes it before creating the directory, so +// admission has to resolve the same segment or it inspects a path that was +// never written. An unusable name is left as-is for the caller to reject. +func workspaceRepositoryEntryName(repoName string) string { + if sanitized := worktree.SanitizeRepoDirName(repoName); sanitized != "" { + return sanitized + } + return repoName +} + func shouldDeferMissingWorktreeResumeValidation(req *LaunchRequest, workspacePath string) bool { if req == nil || req.ExecutorType != string(models.ExecutorTypeWorktree) || req.ACPSessionID == "" || workspacePath == "" { return false diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_launch_workspace_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_launch_workspace_test.go index 6e98c07df1..a6aece7564 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_launch_workspace_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_launch_workspace_test.go @@ -2,12 +2,14 @@ package lifecycle import ( "context" + "os/exec" "path/filepath" "testing" "github.com/stretchr/testify/require" "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/worktree" v1 "github.com/kandev/kandev/pkg/api/v1" ) @@ -139,6 +141,66 @@ func TestValidateLaunchWorkspaceAdmissionDefersMissingWorktreeResume(t *testing. } } +// TestValidateLaunchWorkspaceAdmissionUsesSanitizedRepositoryDirectory pins the +// multi-repo task root layout: launch specs carry the repository display name, +// which the worktree manager sanitized into the directory segment it actually +// created. Admission must look under that same segment. +func TestValidateLaunchWorkspaceAdmissionUsesSanitizedRepositoryDirectory(t *testing.T) { + root := t.TempDir() + first := initGitRepo(t) + second := initGitRepo(t) + addLinkedWorktree(t, first, filepath.Join(root, worktree.SanitizeRepoDirName("kdlbs/kandev"))) + addLinkedWorktree(t, second, filepath.Join(root, worktree.SanitizeRepoDirName("kdlbs/docs"))) + + req := &LaunchRequest{ + ExecutorType: string(models.ExecutorTypeLocal), + Repositories: []RepoLaunchSpec{ + {RepositoryID: "repository-1", RepositoryPath: first, RepoName: "kdlbs/kandev"}, + {RepositoryID: "repository-2", RepositoryPath: second, RepoName: "kdlbs/docs"}, + }, + } + + if err := validateLaunchWorkspaceAdmission(context.Background(), req, root); err != nil { + t.Fatalf("validateLaunchWorkspaceAdmission() rejected a sanitized repository directory: %v", err) + } +} + +// TestValidateLaunchWorkspaceAdmissionRejectsUnrelatedSanitizedWorktreeOnResume +// keeps the resume path fail-closed once the sanitized directory is the one +// inspected: a checkout of a different repository must not be admitted just +// because the raw display name resolves to nothing. +func TestValidateLaunchWorkspaceAdmissionRejectsUnrelatedSanitizedWorktreeOnResume(t *testing.T) { + root := t.TempDir() + first := initGitRepo(t) + second := initGitRepo(t) + unrelated := initGitRepo(t) + addLinkedWorktree(t, first, filepath.Join(root, worktree.SanitizeRepoDirName("kdlbs/kandev"))) + addLinkedWorktree(t, unrelated, filepath.Join(root, worktree.SanitizeRepoDirName("kdlbs/docs"))) + + req := &LaunchRequest{ + ExecutorType: string(models.ExecutorTypeWorktree), + ACPSessionID: "acp-session-1", + Repositories: []RepoLaunchSpec{ + {RepositoryID: "repository-1", RepositoryPath: first, RepoName: "kdlbs/kandev"}, + {RepositoryID: "repository-2", RepositoryPath: second, RepoName: "kdlbs/docs"}, + }, + } + + if err := validateLaunchWorkspaceAdmission(context.Background(), req, root); err == nil { + t.Fatal("validateLaunchWorkspaceAdmission() accepted an unrelated checkout at the sanitized directory") + } +} + +func addLinkedWorktree(t *testing.T, source, destination string) { + t.Helper() + cmd := exec.Command("git", "worktree", "add", "--detach", destination) + cmd.Dir = source + cmd.Env = newIsolatedGitEnv() + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git worktree add %q: %v: %s", destination, err, output) + } +} + func TestTruncateID(t *testing.T) { require.Equal(t, "abc", truncateID("abc", 8)) require.Equal(t, "abcdefgh", truncateID("abcdefgh", 8))