fix(lifecycle): resolve worktree dir by sanitized repo name on launch admission (#3580)
Co-authored-by: Joao Manso <X363081@bcpcorp.net> Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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))
|
||||
|
||||
Reference in New Issue
Block a user