fix: support fork PR links on upstream tasks (#3885)
This commit is contained in:
@@ -158,6 +158,33 @@ func TestGitHubPRBaseResolverUsesLinkedPRForLegacyAttachment(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestGitHubPRBaseResolverUsesAttachedTargetWhenAssociationIsMissing(t *testing.T) {
|
||||
service := &githubPRBaseLookupServiceStub{pr: &githubpkg.PR{
|
||||
Number: 42, RepoOwner: "upstream", RepoName: "widgets",
|
||||
BaseRepoOwner: "upstream", BaseRepoName: "widgets",
|
||||
HeadRepoOwner: "fork-owner", HeadRepoName: "widgets", HeadBranch: "feature",
|
||||
BaseBranch: "main", BaseSHA: "0123456789abcdef0123456789abcdef01234567",
|
||||
}}
|
||||
resolver := githubPRBaseResolver{service: service}
|
||||
lookup := executorpkg.PRBaseLookup{
|
||||
TaskID: "task-1", TaskRepositoryID: "task-repo-1", RepositoryID: "repo-1",
|
||||
Number: 42, CheckoutBranch: "feature", AttachedOwner: "upstream", AttachedRepository: "widgets",
|
||||
}
|
||||
base, err := resolver.ResolvePRBase(context.Background(), "workspace-1", lookup)
|
||||
if err != nil {
|
||||
t.Fatalf("ResolvePRBase() error: %v", err)
|
||||
}
|
||||
if base.Target.TargetRepository.Path != "upstream/widgets" ||
|
||||
base.Target.HeadRepository.Path != "fork-owner/widgets" || base.Target.HeadBranch != "feature" {
|
||||
t.Fatalf("resolved target-attached PR = %#v, want upstream target and validated fork head", base.Target)
|
||||
}
|
||||
if !reflect.DeepEqual(service.getCalls, []string{"upstream/widgets#42"}) ||
|
||||
!reflect.DeepEqual(service.listCalls, [][]string{{"task-1"}}) {
|
||||
t.Fatalf("provider lookups = (%v, %v), want empty association check then attached namespace lookup",
|
||||
service.getCalls, service.listCalls)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGitHubPRBaseResolverClassifiesKnownCrossRepositoryFailure(t *testing.T) {
|
||||
service := &githubPRBaseLookupServiceStub{
|
||||
prs: []*githubpkg.TaskPR{{
|
||||
|
||||
@@ -591,6 +591,27 @@ func (s *Service) attachTaskToReservation(ctx context.Context, evt *github.NewRe
|
||||
// buildReviewTaskRequest builds the ReviewTaskRequest payload from an event.
|
||||
func buildReviewTaskRequest(evt *github.NewReviewPREvent, repositories []ReviewTaskRepository, repoSlug string) *ReviewTaskRequest {
|
||||
pr := evt.PR
|
||||
metadata := map[string]interface{}{
|
||||
"review_watch_id": evt.ReviewWatchID,
|
||||
"pr_number": pr.Number,
|
||||
"pr_url": pr.HTMLURL,
|
||||
"pr_repo": repoSlug,
|
||||
"pr_author": pr.AuthorLogin,
|
||||
"pr_branch": pr.HeadBranch,
|
||||
"agent_profile_id": evt.AgentProfileID,
|
||||
"executor_profile_id": evt.ExecutorProfileID,
|
||||
// Permanent guard: both the workflow-promotion path and this watcher's
|
||||
// synchronous path must compete for the one-shot launch token.
|
||||
models.MetaKeyAutoStartGuard: true,
|
||||
}
|
||||
if reviewPRHeadMatchesBase(pr) {
|
||||
metadata[models.MetaKeyAutoStartClaimed] = true
|
||||
} else {
|
||||
// Fork-controlled files can be evaluated by the repository setup script,
|
||||
// which receives executor-profile environment values. Leave fork review
|
||||
// tasks for an explicit user start instead of granting an unattended token.
|
||||
metadata[models.MetaKeyForkPRRequiresManualStart] = true
|
||||
}
|
||||
return &ReviewTaskRequest{
|
||||
WorkspaceID: evt.WorkspaceID,
|
||||
WorkflowID: evt.WorkflowID,
|
||||
@@ -598,29 +619,17 @@ func buildReviewTaskRequest(evt *github.NewReviewPREvent, repositories []ReviewT
|
||||
Title: service.TruncateTaskTitle(fmt.Sprintf("PR #%d: %s", pr.Number, pr.Title)),
|
||||
Description: interpolateReviewPrompt(evt.Prompt, pr),
|
||||
Repositories: repositories,
|
||||
Metadata: map[string]interface{}{
|
||||
"review_watch_id": evt.ReviewWatchID,
|
||||
"pr_number": pr.Number,
|
||||
"pr_url": pr.HTMLURL,
|
||||
"pr_repo": repoSlug,
|
||||
"pr_author": pr.AuthorLogin,
|
||||
"pr_branch": pr.HeadBranch,
|
||||
"agent_profile_id": evt.AgentProfileID,
|
||||
"executor_profile_id": evt.ExecutorProfileID,
|
||||
// Permanent guard: signals that the auto-start idempotency protocol
|
||||
// is active for this task. Both Path A (promotion, async) and Path B
|
||||
// (watcher, sync) check this marker and then compete for the one-shot
|
||||
// MetaKeyAutoStartClaimed token before calling StartTask.
|
||||
models.MetaKeyAutoStartGuard: true,
|
||||
// One-shot token: both paths atomically remove this key; only the
|
||||
// first removal wins and proceeds to StartTask. Prevents the
|
||||
// duplicate-agent bug when CreateTask synchronously promotes the task
|
||||
// from its feeder into an auto-start destination.
|
||||
models.MetaKeyAutoStartClaimed: true,
|
||||
},
|
||||
Metadata: metadata,
|
||||
}
|
||||
}
|
||||
|
||||
func reviewPRHeadMatchesBase(pr *github.PR) bool {
|
||||
if pr == nil || pr.RepoOwner == "" || pr.RepoName == "" || pr.HeadRepoOwner == "" || pr.HeadRepoName == "" {
|
||||
return false
|
||||
}
|
||||
return strings.EqualFold(pr.RepoOwner, pr.HeadRepoOwner) && strings.EqualFold(pr.RepoName, pr.HeadRepoName)
|
||||
}
|
||||
|
||||
// buildIssueTaskTitle builds the "Issue #<n>: <title>" task title for an issue
|
||||
// watcher, shortened to the task title limit so an over-long remote issue title
|
||||
// does not cause CreateIssueTask to reject the whole task.
|
||||
@@ -646,6 +655,11 @@ func (s *Service) shouldAutoStartStep(ctx context.Context, stepID string) bool {
|
||||
func (s *Service) autoStartReviewTask(
|
||||
ctx context.Context, evt *github.NewReviewPREvent, task *models.Task,
|
||||
) {
|
||||
if taskRequiresManualForkPRStart(task) {
|
||||
s.logger.Info("fork review task is waiting for a manual start",
|
||||
zap.String("task_id", task.ID), zap.Int("pr_number", evt.PR.Number))
|
||||
return
|
||||
}
|
||||
if s.shouldSkipTerminalPRAutoStart(ctx, task) {
|
||||
return
|
||||
}
|
||||
@@ -655,7 +669,7 @@ func (s *Service) autoStartReviewTask(
|
||||
// autoStartTaskForStep (Path A) asynchronously; only the first claimer
|
||||
// launches so exactly one session is created for the task.
|
||||
if !s.claimAutoStart(ctx, task.ID, "review.auto_start") {
|
||||
s.logger.Debug("review auto-start claim lost; promotion path will launch",
|
||||
s.logger.Debug("review auto-start claim unavailable; skipping automatic launch",
|
||||
zap.String("task_id", task.ID),
|
||||
zap.Int("pr_number", evt.PR.Number))
|
||||
return
|
||||
|
||||
@@ -80,6 +80,39 @@ func TestBuildReviewTaskRequest_TruncatesTitle(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildReviewTaskRequest_ForkPRRequiresManualStart(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
headOwner string
|
||||
headRepo string
|
||||
wantAutostart bool
|
||||
}{
|
||||
{name: "same repository", headOwner: "acme", headRepo: "widget", wantAutostart: true},
|
||||
{name: "fork repository", headOwner: "contributor", headRepo: "widget"},
|
||||
{name: "missing head repository identity"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
evt := newReviewEvent()
|
||||
evt.PR.HeadRepoOwner = tt.headOwner
|
||||
evt.PR.HeadRepoName = tt.headRepo
|
||||
|
||||
req := buildReviewTaskRequest(evt, nil, "acme/widget")
|
||||
if req.Metadata[taskmodels.MetaKeyAutoStartGuard] != true {
|
||||
t.Fatalf("auto-start guard = %v, want true", req.Metadata[taskmodels.MetaKeyAutoStartGuard])
|
||||
}
|
||||
_, gotAutostart := req.Metadata[taskmodels.MetaKeyAutoStartClaimed]
|
||||
if gotAutostart != tt.wantAutostart {
|
||||
t.Errorf("automatic launch token present = %v, want %v", gotAutostart, tt.wantAutostart)
|
||||
}
|
||||
_, requiresManualStart := req.Metadata[taskmodels.MetaKeyForkPRRequiresManualStart]
|
||||
if requiresManualStart != !tt.wantAutostart {
|
||||
t.Errorf("manual-start marker present = %v, want %v", requiresManualStart, !tt.wantAutostart)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestBuildIssueTaskTitle_TruncatesTitle mirrors the review-title regression for
|
||||
// the GitHub issue watcher's "Issue #<n>: <title>" builder.
|
||||
func TestBuildIssueTaskTitle_TruncatesTitle(t *testing.T) {
|
||||
@@ -175,6 +208,7 @@ func newReviewEvent() *github.NewReviewPREvent {
|
||||
PR: &github.PR{
|
||||
Number: 42, Title: "Some PR", HTMLURL: "https://gh/acme/widget/pull/42",
|
||||
RepoOwner: "acme", RepoName: "widget",
|
||||
HeadRepoOwner: "acme", HeadRepoName: "widget",
|
||||
},
|
||||
}
|
||||
}
|
||||
@@ -434,6 +468,110 @@ func TestAutoStart_BothPathsFireExactlyOnce(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestAutoStart_ForkReviewWaitsForManualStart(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
const taskID = "task-fork-review-42"
|
||||
const stepID = "step-fork-review"
|
||||
|
||||
sg := newMockStepGetter()
|
||||
sg.steps[stepID] = &wfmodels.WorkflowStep{
|
||||
ID: stepID, WorkflowID: "wf1", Name: "Review", Position: 0,
|
||||
Events: wfmodels.StepEvents{
|
||||
OnEnter: []wfmodels.OnEnterAction{{Type: wfmodels.OnEnterAutoStartAgent}},
|
||||
},
|
||||
}
|
||||
evt := newReviewEvent()
|
||||
evt.WorkflowStepID = stepID
|
||||
evt.PR.HeadRepoOwner = "contributor"
|
||||
request := buildReviewTaskRequest(evt, nil, "acme/widget")
|
||||
if _, hasToken := request.Metadata[taskmodels.MetaKeyAutoStartClaimed]; hasToken {
|
||||
t.Fatal("fork review task must not carry an unattended auto-start token")
|
||||
}
|
||||
if request.Metadata[taskmodels.MetaKeyForkPRRequiresManualStart] != true {
|
||||
t.Fatal("fork review task must persist the manual-start requirement")
|
||||
}
|
||||
|
||||
now := time.Now().UTC()
|
||||
if err := repo.CreateWorkspace(ctx, &taskmodels.Workspace{ID: "ws1", Name: "Test", CreatedAt: now, UpdatedAt: now}); err != nil {
|
||||
t.Fatalf("CreateWorkspace: %v", err)
|
||||
}
|
||||
if err := repo.CreateWorkflow(ctx, &taskmodels.Workflow{ID: "wf1", WorkspaceID: "ws1", Name: "WF", CreatedAt: now, UpdatedAt: now}); err != nil {
|
||||
t.Fatalf("CreateWorkflow: %v", err)
|
||||
}
|
||||
metadata := make(map[string]interface{}, len(request.Metadata)+1)
|
||||
for key, value := range request.Metadata {
|
||||
metadata[key] = value
|
||||
}
|
||||
metadata[taskmodels.MetaKeyAgentProfileID] = testAgentProfileID
|
||||
task := &taskmodels.Task{
|
||||
ID: taskID, WorkspaceID: "ws1", WorkflowID: "wf1", WorkflowStepID: stepID,
|
||||
Title: request.Title, Description: request.Description, State: v1.TaskStateInProgress,
|
||||
Metadata: metadata, CreatedAt: now, UpdatedAt: now,
|
||||
}
|
||||
if err := repo.CreateTask(ctx, task); err != nil {
|
||||
t.Fatalf("CreateTask: %v", err)
|
||||
}
|
||||
|
||||
taskRepo := newMockTaskRepo()
|
||||
taskRepo.tasks[taskID] = &v1.Task{
|
||||
ID: taskID, State: v1.TaskStateInProgress,
|
||||
Metadata: map[string]interface{}{
|
||||
taskmodels.MetaKeyAutoStartGuard: true,
|
||||
taskmodels.MetaKeyAgentProfileID: testAgentProfileID,
|
||||
},
|
||||
}
|
||||
var launchCount atomic.Int32
|
||||
launched := make(chan struct{}, 2)
|
||||
agentMgr := &mockAgentManager{
|
||||
repoForExecutionLookup: repo,
|
||||
launchAgentFunc: func(_ context.Context, _ *executor.LaunchAgentRequest) (*executor.LaunchAgentResponse, error) {
|
||||
launchCount.Add(1)
|
||||
launched <- struct{}{}
|
||||
return &executor.LaunchAgentResponse{}, nil
|
||||
},
|
||||
}
|
||||
svc := createTestServiceWithScheduler(repo, sg, taskRepo, agentMgr)
|
||||
|
||||
dbTask, err := repo.GetTask(ctx, taskID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetTask: %v", err)
|
||||
}
|
||||
svc.autoStartReviewTask(ctx, evt, dbTask)
|
||||
svc.autoStartTaskForStep(ctx, taskID, stepID, "task.queue_promoted", 0, false)
|
||||
svc.autoStartTaskForLoadedStep(ctx, dbTask, sg.steps[stepID], "task.moved", false, 0, false)
|
||||
select {
|
||||
case <-launched:
|
||||
t.Fatal("fork review task launched automatically")
|
||||
case <-time.After(250 * time.Millisecond):
|
||||
}
|
||||
if got := launchCount.Load(); got != 0 {
|
||||
t.Fatalf("automatic launch count = %d, want 0", got)
|
||||
}
|
||||
sessions, err := repo.ListTaskSessions(ctx, taskID)
|
||||
if err != nil {
|
||||
t.Fatalf("ListTaskSessions before manual start: %v", err)
|
||||
}
|
||||
if len(sessions) != 0 {
|
||||
t.Fatalf("sessions before manual start = %d, want 0", len(sessions))
|
||||
}
|
||||
if _, err := svc.StartTask(ctx, taskID, testAgentProfileID, "", "", "", "Review", stepID, false, true, nil); err != errForkPRManualStartRequired {
|
||||
t.Fatalf("automatic StartTask error = %v, want %v", err, errForkPRManualStartRequired)
|
||||
}
|
||||
|
||||
if _, err := svc.StartTask(ctx, taskID, testAgentProfileID, "", "", "", "Review", stepID, false, false, nil); err != nil {
|
||||
t.Fatalf("manual StartTask: %v", err)
|
||||
}
|
||||
select {
|
||||
case <-launched:
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("manual start did not launch the fork review task")
|
||||
}
|
||||
if got := launchCount.Load(); got != 1 {
|
||||
t.Fatalf("launch count after manual start = %d, want 1", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAutoStart_ClaimIsAtomic verifies that two competing calls to claimAutoStart
|
||||
// for the same task produce exactly one winner.
|
||||
func TestAutoStart_ClaimIsAtomic(t *testing.T) {
|
||||
|
||||
@@ -1716,6 +1716,9 @@ func (s *Service) autoStartTaskForStep(ctx context.Context, taskID, stepID, even
|
||||
}
|
||||
return
|
||||
}
|
||||
if s.skipForkPRAutoStart(ctx, task, eventName, autoStartOnCreateClaimed) {
|
||||
return
|
||||
}
|
||||
if task.QueuedForStepID != "" {
|
||||
if autoStartOnCreateClaimed {
|
||||
s.restoreAutoStartOnCreate(ctx, taskID, eventName)
|
||||
@@ -1948,6 +1951,9 @@ func (s *Service) autoStartTaskForLoadedStep(ctx context.Context, task *models.T
|
||||
}
|
||||
return
|
||||
}
|
||||
if s.skipForkPRAutoStart(ctx, task, eventName, autoStartOnCreateClaimed) {
|
||||
return
|
||||
}
|
||||
if models.HasAutoStartOnCreateIntent(task.Metadata) || models.HasAutoStartOnCreateInFlight(task.Metadata) || autoStartOnCreateClaimed {
|
||||
sessions, err := s.repo.ListTaskSessions(ctx, task.ID)
|
||||
if err != nil {
|
||||
@@ -2097,6 +2103,26 @@ func (s *Service) autoStartTaskForLoadedStep(ctx context.Context, task *models.T
|
||||
}()
|
||||
}
|
||||
|
||||
func taskRequiresManualForkPRStart(task *models.Task) bool {
|
||||
if task == nil || task.Metadata == nil {
|
||||
return false
|
||||
}
|
||||
required, _ := task.Metadata[models.MetaKeyForkPRRequiresManualStart].(bool)
|
||||
return required
|
||||
}
|
||||
|
||||
func (s *Service) skipForkPRAutoStart(ctx context.Context, task *models.Task, eventName string, autoStartOnCreateClaimed bool) bool {
|
||||
if !taskRequiresManualForkPRStart(task) {
|
||||
return false
|
||||
}
|
||||
s.logger.Info(eventName+": fork review task is waiting for a manual start",
|
||||
zap.String("task_id", task.ID))
|
||||
if autoStartOnCreateClaimed || models.HasAutoStartOnCreateIntent(task.Metadata) || models.HasAutoStartOnCreateInFlight(task.Metadata) {
|
||||
s.discardAutoStartOnCreate(ctx, task.ID, eventName)
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
// autoStartOfficeTaskForLoadedStep is the Office-aware counterpart of the
|
||||
// kanban launch path above. An Office task has no session-based launch route
|
||||
// (see validateOfficeRuntimeEnv) — StartTask can never succeed for it, it
|
||||
|
||||
@@ -2,6 +2,7 @@ package executor
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
|
||||
"github.com/kandev/kandev/internal/task/models"
|
||||
@@ -73,6 +74,205 @@ func TestResolveTaskRepoInfo_PRBaseRejectsDifferentHeadForSameNumber(t *testing.
|
||||
}
|
||||
}
|
||||
|
||||
// @covers AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.11
|
||||
// @covers AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.15
|
||||
func TestResolveTaskRepoInfo_TargetAttachedForkPRWithoutContribution(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
repo.repositories["repo-1"] = &models.Repository{
|
||||
ID: "repo-1", WorkspaceID: "workspace-1", SourceType: sourceTypeLocal,
|
||||
LocalPath: t.TempDir(), Provider: "github", ProviderOwner: "upstream",
|
||||
ProviderName: "widgets", DefaultBranch: "main",
|
||||
}
|
||||
target := forkPRComparisonTarget()
|
||||
target.TargetBranch = "main"
|
||||
resolver := &recordingPRBaseResolver{result: &models.PRBase{
|
||||
Target: *target, OID: "0123456789abcdef0123456789abcdef01234567",
|
||||
}}
|
||||
exec := newTestExecutor(t, &mockAgentManager{}, repo)
|
||||
exec.SetPRBaseResolver(resolver)
|
||||
|
||||
info, err := exec.resolveTaskRepoInfo(context.Background(), &models.TaskRepository{
|
||||
ID: "task-repo-1", TaskID: "task-1", RepositoryID: "repo-1",
|
||||
BaseBranch: "main", CheckoutBranch: "feature",
|
||||
Metadata: map[string]interface{}{"pr_number": 42},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("resolveTaskRepoInfo() error: %v", err)
|
||||
}
|
||||
if info.PRBase == nil || info.PRBase.Target.HeadRepository.Path != "fork-owner/widgets" ||
|
||||
info.BaseBranch != "main" {
|
||||
t.Fatalf("resolved target-attached fork PR = %#v, want transient fork identity and target base", info)
|
||||
}
|
||||
if info.QualifiedPRBase != nil || info.ComparisonTarget != nil {
|
||||
t.Fatalf("ordinary target-attached PR added qualified metadata: base=%#v target=%#v",
|
||||
info.QualifiedPRBase, info.ComparisonTarget)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveTaskRepoInfo_TargetAttachedForkPRRejectsInvalidProviderIdentity(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
checkoutBranch string
|
||||
mutate func(*models.ComparisonTarget)
|
||||
}{
|
||||
{name: "unrelated target", checkoutBranch: "feature", mutate: func(target *models.ComparisonTarget) {
|
||||
target.TargetRepository = models.ComparisonTargetRepository{
|
||||
Host: "github.com", Path: "other/widgets", RemoteURL: "https://github.com/other/widgets.git",
|
||||
}
|
||||
}},
|
||||
{name: "wrong PR number", checkoutBranch: "feature", mutate: func(target *models.ComparisonTarget) {
|
||||
target.Number = 43
|
||||
}},
|
||||
{name: "wrong head branch", checkoutBranch: "feature", mutate: func(target *models.ComparisonTarget) {
|
||||
target.HeadBranch = "other"
|
||||
}},
|
||||
{name: "missing head repository", checkoutBranch: "feature", mutate: func(target *models.ComparisonTarget) {
|
||||
target.HeadRepository = models.ComparisonTargetRepository{}
|
||||
}},
|
||||
{name: "empty checkout branch", checkoutBranch: "", mutate: func(*models.ComparisonTarget) {}},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
repo.repositories["repo-1"] = &models.Repository{
|
||||
ID: "repo-1", WorkspaceID: "workspace-1", SourceType: sourceTypeLocal,
|
||||
LocalPath: t.TempDir(), Provider: "github", ProviderOwner: "upstream",
|
||||
ProviderName: "widgets", DefaultBranch: "main",
|
||||
}
|
||||
target := forkPRComparisonTarget()
|
||||
target.TargetBranch = "main"
|
||||
tt.mutate(target)
|
||||
resolver := &recordingPRBaseResolver{result: &models.PRBase{Target: *target}}
|
||||
exec := newTestExecutor(t, &mockAgentManager{}, repo)
|
||||
exec.SetPRBaseResolver(resolver)
|
||||
_, err := exec.resolveTaskRepoInfo(context.Background(), &models.TaskRepository{
|
||||
ID: "task-repo-1", TaskID: "task-1", RepositoryID: "repo-1",
|
||||
BaseBranch: "main", CheckoutBranch: tt.checkoutBranch,
|
||||
Metadata: map[string]interface{}{"pr_number": 42},
|
||||
})
|
||||
if err == nil {
|
||||
t.Fatal("resolveTaskRepoInfo() accepted an invalid target-attached PR identity")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveTaskRepoInfo_TargetAttachedForkPRDoesNotOverrideExplicitBindings(t *testing.T) {
|
||||
t.Run("comparison target", func(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
repo.repositories["repo-1"] = targetAttachedPRRepository(t)
|
||||
expected := forkPRComparisonTarget()
|
||||
metadata := map[string]interface{}{"pr_number": 42}
|
||||
if err := models.PutComparisonTarget(metadata, expected); err != nil {
|
||||
t.Fatalf("PutComparisonTarget() error: %v", err)
|
||||
}
|
||||
resolved := *expected
|
||||
resolved.TargetBranch = "live-main"
|
||||
resolved.HeadRepository = models.ComparisonTargetRepository{
|
||||
Host: "github.com", Path: "other-fork/widgets", RemoteURL: "https://github.com/other-fork/widgets.git",
|
||||
}
|
||||
resolver := &recordingPRBaseResolver{result: &models.PRBase{Target: resolved}}
|
||||
exec := newTestExecutor(t, &mockAgentManager{}, repo)
|
||||
exec.SetPRBaseResolver(resolver)
|
||||
info, err := exec.resolveTaskRepoInfo(context.Background(), &models.TaskRepository{
|
||||
ID: "task-repo-1", TaskID: "task-1", RepositoryID: "repo-1",
|
||||
BaseBranch: "main", CheckoutBranch: "feature", Metadata: metadata,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("resolveTaskRepoInfo() error: %v", err)
|
||||
}
|
||||
if info.BaseBranch != expected.TargetBranch || info.PRBase == nil ||
|
||||
!info.PRBase.Target.Equal(*expected) {
|
||||
t.Fatalf("mismatched provider data replaced comparison binding: %#v", info)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("contribution source", func(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
repo.repositories["repo-1"] = targetAttachedPRRepository(t)
|
||||
metadata := map[string]interface{}{"pr_number": 42}
|
||||
binding := &models.RemoteContribution{
|
||||
Version: models.RemoteContributionVersion, Provider: models.RemoteContributionProviderGitHub,
|
||||
Kind: models.RemoteContributionKindPullRequest, CanonicalURL: "https://github.com/upstream/widgets/pull/42",
|
||||
Number: 42, State: models.RemoteContributionStateOpen, BaseBranch: "main", HeadBranch: "feature",
|
||||
HeadSHA: "0123456789abcdef0123456789abcdef01234567",
|
||||
SourceRepository: models.RemoteContributionRepository{
|
||||
Host: "github.com", Path: "fork-owner/widgets", RemoteURL: "https://github.com/fork-owner/widgets.git",
|
||||
},
|
||||
}
|
||||
if err := models.PutRemoteContribution(metadata, binding); err != nil {
|
||||
t.Fatalf("PutRemoteContribution() error: %v", err)
|
||||
}
|
||||
target := forkPRComparisonTarget()
|
||||
target.TargetBranch = "main"
|
||||
target.HeadRepository = models.ComparisonTargetRepository{
|
||||
Host: "github.com", Path: "other-fork/widgets", RemoteURL: "https://github.com/other-fork/widgets.git",
|
||||
}
|
||||
resolver := &recordingPRBaseResolver{result: &models.PRBase{Target: *target}}
|
||||
exec := newTestExecutor(t, &mockAgentManager{}, repo)
|
||||
exec.SetPRBaseResolver(resolver)
|
||||
if _, err := exec.resolveTaskRepoInfo(context.Background(), &models.TaskRepository{
|
||||
ID: "task-repo-1", TaskID: "task-1", RepositoryID: "repo-1",
|
||||
BaseBranch: "main", CheckoutBranch: "feature", Metadata: metadata,
|
||||
}); err == nil {
|
||||
t.Fatal("resolveTaskRepoInfo() accepted a provider head outside the contribution source binding")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestResolveTaskRepoInfo_TargetAttachedForkPRPreservesCancellation(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
repo.repositories["repo-1"] = targetAttachedPRRepository(t)
|
||||
exec := newTestExecutor(t, &mockAgentManager{}, repo)
|
||||
exec.SetPRBaseResolver(&recordingPRBaseResolver{err: context.Canceled})
|
||||
info, err := exec.resolveTaskRepoInfo(context.Background(), &models.TaskRepository{
|
||||
ID: "task-repo-1", TaskID: "task-1", RepositoryID: "repo-1",
|
||||
BaseBranch: "main", CheckoutBranch: "feature", Metadata: map[string]interface{}{"pr_number": 42},
|
||||
})
|
||||
if info != nil || !errors.Is(err, context.Canceled) {
|
||||
t.Fatalf("resolveTaskRepoInfo() = (%#v, %v), want cancellation", info, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveAllRepoInfoRejectsLaunchWhenOnePRBindingIsInvalid(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
for id, owner := range map[string]string{"repo-1": "upstream", "repo-2": "other"} {
|
||||
repo.repositories[id] = &models.Repository{
|
||||
ID: id, WorkspaceID: "workspace-1", SourceType: sourceTypeLocal,
|
||||
LocalPath: t.TempDir(), Provider: "github", ProviderOwner: owner,
|
||||
ProviderName: "widgets", DefaultBranch: "main",
|
||||
}
|
||||
}
|
||||
for i, id := range []string{"repo-1", "repo-2"} {
|
||||
repo.taskRepositories[id] = &models.TaskRepository{
|
||||
ID: "task-repo-" + id, TaskID: "task-1", RepositoryID: id,
|
||||
Position: i, BaseBranch: "main", CheckoutBranch: "feature",
|
||||
Metadata: map[string]interface{}{"pr_number": 42},
|
||||
}
|
||||
}
|
||||
target := forkPRComparisonTarget()
|
||||
target.TargetBranch = "main"
|
||||
resolver := &recordingPRBaseResolver{result: &models.PRBase{Target: *target}}
|
||||
exec := newTestExecutor(t, &mockAgentManager{}, repo)
|
||||
exec.SetPRBaseResolver(resolver)
|
||||
infos, err := exec.resolveAllRepoInfo(context.Background(), "task-1")
|
||||
if err == nil || infos != nil {
|
||||
t.Fatalf("resolveAllRepoInfo() = (%#v, %v), want whole launch rejected", infos, err)
|
||||
}
|
||||
if len(resolver.calls) != 2 {
|
||||
t.Fatalf("resolver calls = %d, want both repository bindings checked", len(resolver.calls))
|
||||
}
|
||||
}
|
||||
|
||||
func targetAttachedPRRepository(t *testing.T) *models.Repository {
|
||||
t.Helper()
|
||||
return &models.Repository{
|
||||
ID: "repo-1", WorkspaceID: "workspace-1", SourceType: sourceTypeLocal,
|
||||
LocalPath: t.TempDir(), Provider: "github", ProviderOwner: "upstream",
|
||||
ProviderName: "widgets", DefaultBranch: "main",
|
||||
}
|
||||
}
|
||||
|
||||
// @covers AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.15
|
||||
func TestResolveTaskRepoInfo_LegacyForkPRQualifiesLiveLinkedBaseForMaterialization(t *testing.T) {
|
||||
target := forkPRComparisonTarget()
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"reflect"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
@@ -91,6 +92,135 @@ func TestLegacyLinkedPRBaseFlowsIntoWorktreeMaterialization(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestTargetAttachedForkPRBasePreparationEndToEnd(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
gitConfig := filepath.Join(root, "empty-git-config")
|
||||
if err := os.WriteFile(gitConfig, nil, 0o600); err != nil {
|
||||
t.Fatalf("create isolated Git config: %v", err)
|
||||
}
|
||||
t.Setenv("GIT_CONFIG_GLOBAL", gitConfig)
|
||||
t.Setenv("GIT_CONFIG_NOSYSTEM", "1")
|
||||
repoPath, targetOID, forkMainOID, prHeadOID := initTargetAttachedForkPRMaterializationRepos(t, root)
|
||||
if targetOID == forkMainOID {
|
||||
t.Fatal("upstream and fork main branches unexpectedly share an OID")
|
||||
}
|
||||
|
||||
target := forkPRComparisonTarget()
|
||||
target.HeadBranch = "feature/pr-42"
|
||||
target.TargetBranch = "main"
|
||||
resolver := &recordingPRBaseResolver{result: &models.PRBase{
|
||||
Target: *target, OID: targetOID,
|
||||
}}
|
||||
repositoryStore := newMockRepository()
|
||||
repositoryStore.repositories["repo-1"] = &models.Repository{
|
||||
ID: "repo-1", WorkspaceID: "workspace-1", Name: "widgets", SourceType: sourceTypeLocal,
|
||||
LocalPath: repoPath, Provider: "github", ProviderOwner: "upstream", ProviderName: "widgets",
|
||||
DefaultBranch: "main",
|
||||
}
|
||||
exec := newTestExecutor(t, &mockAgentManager{}, repositoryStore)
|
||||
exec.SetPRBaseResolver(resolver)
|
||||
taskRepo := &models.TaskRepository{
|
||||
ID: "task-repo-1", TaskID: "task-1", RepositoryID: "repo-1", BaseBranch: "main",
|
||||
CheckoutBranch: "feature/pr-42", Metadata: map[string]interface{}{"pr_number": 42},
|
||||
}
|
||||
info, err := exec.resolveTaskRepoInfo(context.Background(), taskRepo)
|
||||
if err != nil {
|
||||
t.Fatalf("resolve target-attached PR task: %v", err)
|
||||
}
|
||||
retriedInfo, err := exec.resolveTaskRepoInfo(context.Background(), taskRepo)
|
||||
if err != nil {
|
||||
t.Fatalf("retry target-attached PR task before preparation: %v", err)
|
||||
}
|
||||
if retriedInfo.PRBase == nil || info.PRBase == nil || !reflect.DeepEqual(retriedInfo.PRBase, info.PRBase) {
|
||||
t.Fatalf("retried PR identity = %#v, want unchanged provider identity %#v", retriedInfo.PRBase, info.PRBase)
|
||||
}
|
||||
info = retriedInfo
|
||||
if info.PRBase == nil || info.PRBase.Target.HeadRepository.Path != "fork-owner/widgets" ||
|
||||
info.QualifiedPRBase != nil || info.ComparisonTarget != nil {
|
||||
t.Fatalf("resolved PR identity = %#v, want transient fork source without a qualified target", info)
|
||||
}
|
||||
specs := buildRepoSpecs([]*repoInfo{info})
|
||||
if len(specs) != 1 || specs[0].BaseBranch != "main" || specs[0].PRNumber != 42 || specs[0].QualifiedPRBase != nil {
|
||||
t.Fatalf("target-attached repository spec = %#v, want ordinary main + PR-head preparation", specs)
|
||||
}
|
||||
log, err := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json"})
|
||||
if err != nil {
|
||||
t.Fatalf("create test logger: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = log.Close() })
|
||||
mgr, err := worktree.NewManager(worktree.Config{
|
||||
Enabled: true, TasksBasePath: filepath.Join(root, "tasks"), BranchPrefix: "kandev/",
|
||||
}, nil, log)
|
||||
if err != nil {
|
||||
t.Fatalf("create worktree manager: %v", err)
|
||||
}
|
||||
prepared, err := lifecycle.NewWorktreePreparer(mgr, log).Prepare(context.Background(), &lifecycle.EnvPrepareRequest{
|
||||
TaskID: "task-1", WorkspaceID: "workspace-1", SessionID: "session-1", TaskDirName: "task-1",
|
||||
RepositoryID: specs[0].RepositoryID, TaskRepositoryID: specs[0].TaskRepositoryID,
|
||||
RepositoryPath: specs[0].RepositoryPath, RepoName: specs[0].RepoName,
|
||||
UseWorktree: true, BaseBranch: specs[0].BaseBranch, DefaultBranch: specs[0].DefaultBranch,
|
||||
CheckoutBranch: specs[0].CheckoutBranch, PRNumber: specs[0].PRNumber,
|
||||
}, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("prepare target-attached fork PR worktree: %v", err)
|
||||
}
|
||||
if !prepared.Success || prepared.BaseBranch != "main" {
|
||||
t.Fatalf("prepared target-attached PR worktree = %#v, want successful main-based preparation", prepared)
|
||||
}
|
||||
if got := gitOutputInTest(t, prepared.WorkspacePath, "rev-parse", "HEAD"); got != prHeadOID {
|
||||
t.Fatalf("prepared HEAD = %q, want fork PR head %q", got, prHeadOID)
|
||||
}
|
||||
if got := gitOutputInTest(t, repoPath, "rev-parse", "refs/remotes/origin/main"); got != targetOID {
|
||||
t.Fatalf("origin/main = %q, want attached upstream OID %q", got, targetOID)
|
||||
}
|
||||
if got := gitOutputInTest(t, repoPath, "config", "--get", "remote.origin.url"); got != filepath.Join(root, "upstream.git") {
|
||||
t.Fatalf("origin URL = %q, want unchanged upstream remote", got)
|
||||
}
|
||||
if got := gitOutputInTest(t, repoPath, "config", "--get", "remote.origin.pushurl"); got != "https://push.example/upstream/widgets.git" {
|
||||
t.Fatalf("origin push URL = %q, want unchanged upstream push route", got)
|
||||
}
|
||||
}
|
||||
|
||||
func initTargetAttachedForkPRMaterializationRepos(
|
||||
t *testing.T, root string,
|
||||
) (repoPath, upstreamMainOID, forkMainOID, prHeadOID string) {
|
||||
t.Helper()
|
||||
upstreamBare := filepath.Join(root, "upstream.git")
|
||||
forkBare := filepath.Join(root, "fork.git")
|
||||
upstreamSeed := filepath.Join(root, "upstream-seed")
|
||||
forkSeed := filepath.Join(root, "fork-seed")
|
||||
gitCommandInTest(t, root, "init", "--bare", "-b", "main", upstreamBare)
|
||||
gitCommandInTest(t, root, "init", "-b", "main", upstreamSeed)
|
||||
configureLegacyForkPRCommitter(t, upstreamSeed)
|
||||
writeLegacyForkPRFile(t, upstreamSeed, "base.txt", "upstream main\n")
|
||||
gitCommandInTest(t, upstreamSeed, "add", "base.txt")
|
||||
gitCommandInTest(t, upstreamSeed, "commit", "-m", "upstream main")
|
||||
gitCommandInTest(t, upstreamSeed, "remote", "add", "origin", upstreamBare)
|
||||
gitCommandInTest(t, upstreamSeed, "push", "origin", "main")
|
||||
upstreamMainOID = gitOutputInTest(t, upstreamSeed, "rev-parse", "HEAD")
|
||||
gitCommandInTest(t, root, "init", "--bare", "-b", "main", forkBare)
|
||||
gitCommandInTest(t, root, "init", "-b", "main", forkSeed)
|
||||
configureLegacyForkPRCommitter(t, forkSeed)
|
||||
writeLegacyForkPRFile(t, forkSeed, "base.txt", "fork main\n")
|
||||
gitCommandInTest(t, forkSeed, "add", "base.txt")
|
||||
gitCommandInTest(t, forkSeed, "commit", "-m", "fork main")
|
||||
gitCommandInTest(t, forkSeed, "remote", "add", "origin", forkBare)
|
||||
gitCommandInTest(t, forkSeed, "push", "origin", "main")
|
||||
forkMainOID = gitOutputInTest(t, forkSeed, "rev-parse", "HEAD")
|
||||
gitCommandInTest(t, forkSeed, "checkout", "-b", "feature/pr-42")
|
||||
writeLegacyForkPRFile(t, forkSeed, "pr.txt", "fork pull request head\n")
|
||||
gitCommandInTest(t, forkSeed, "add", "pr.txt")
|
||||
gitCommandInTest(t, forkSeed, "commit", "-m", "fork pull request head")
|
||||
prHeadOID = gitOutputInTest(t, forkSeed, "rev-parse", "HEAD")
|
||||
gitCommandInTest(t, forkSeed, "push", "origin", "feature/pr-42")
|
||||
gitCommandInTest(t, forkSeed, "remote", "add", "upstream", upstreamBare)
|
||||
gitCommandInTest(t, forkSeed, "push", "upstream", "HEAD:refs/pull/42/head")
|
||||
repoPath = filepath.Join(root, "upstream-checkout")
|
||||
gitCommandInTest(t, root, "clone", upstreamBare, repoPath)
|
||||
gitCommandInTest(t, repoPath, "config", "remote.origin.pushurl", "https://push.example/upstream/widgets.git")
|
||||
return repoPath, upstreamMainOID, forkMainOID, prHeadOID
|
||||
}
|
||||
|
||||
func initLegacyForkPRMaterializationRepos(t *testing.T, root string) (repoPath, upstreamBaseOID, prHeadOID string) {
|
||||
t.Helper()
|
||||
upstreamBare := filepath.Join(root, "upstream.git")
|
||||
|
||||
@@ -418,6 +418,19 @@ func validPRBaseIdentity(
|
||||
if checkoutBranch != "" && target.HeadBranch != checkoutBranch {
|
||||
return false
|
||||
}
|
||||
if expected != nil || contribution {
|
||||
return validBoundPRBaseIdentity(target, expected, attachedRepository, hasAttachedRepository,
|
||||
headRepository, hasHeadRepository, contribution)
|
||||
}
|
||||
return validUnboundPRBaseIdentity(target, checkoutBranch, attachedRepository, hasAttachedRepository,
|
||||
headRepository, hasHeadRepository)
|
||||
}
|
||||
|
||||
func validBoundPRBaseIdentity(
|
||||
target models.ComparisonTarget, expected *models.ComparisonTarget,
|
||||
attachedRepository models.ComparisonTargetRepository, hasAttachedRepository bool,
|
||||
headRepository models.ComparisonTargetRepository, hasHeadRepository, contribution bool,
|
||||
) bool {
|
||||
if !hasAttachedRepository || !hasHeadRepository ||
|
||||
!models.ComparisonTargetRepositoriesEqual(target.HeadRepository, headRepository) {
|
||||
return false
|
||||
@@ -433,6 +446,24 @@ func validPRBaseIdentity(
|
||||
models.ComparisonTargetRepositoriesEqual(expected.HeadRepository, target.HeadRepository)
|
||||
}
|
||||
|
||||
func validUnboundPRBaseIdentity(
|
||||
target models.ComparisonTarget, checkoutBranch string,
|
||||
attachedRepository models.ComparisonTargetRepository, hasAttachedRepository bool,
|
||||
headRepository models.ComparisonTargetRepository, hasHeadRepository bool,
|
||||
) bool {
|
||||
if !hasAttachedRepository || !hasHeadRepository {
|
||||
return false
|
||||
}
|
||||
if models.ComparisonTargetRepositoriesEqual(target.TargetRepository, attachedRepository) {
|
||||
if models.ComparisonTargetRepositoriesEqual(target.HeadRepository, attachedRepository) {
|
||||
return true
|
||||
}
|
||||
return checkoutBranch != ""
|
||||
}
|
||||
// Fork-attached legacy tasks bind the attached repository to the PR head.
|
||||
return models.ComparisonTargetRepositoriesEqual(target.HeadRepository, headRepository)
|
||||
}
|
||||
|
||||
func githubComparisonRepositoryFromRepository(repo *models.Repository) (models.ComparisonTargetRepository, bool) {
|
||||
if repo == nil {
|
||||
return models.ComparisonTargetRepository{}, false
|
||||
|
||||
@@ -1452,6 +1452,8 @@ func (s *Service) startTask(ctx context.Context, taskID string, agentProfileID s
|
||||
if autoStart {
|
||||
if task, taskErr := s.repo.GetTask(ctx, taskID); taskErr != nil {
|
||||
return nil, fmt.Errorf("failed to fetch task for automatic launch gate: %w", taskErr)
|
||||
} else if taskRequiresManualForkPRStart(task) {
|
||||
return nil, errForkPRManualStartRequired
|
||||
} else if s.shouldSkipTerminalPRAutoStart(ctx, task) {
|
||||
return nil, nil
|
||||
}
|
||||
@@ -1964,6 +1966,7 @@ func validateOfficeRuntimeEnv(env map[string]string) error {
|
||||
}
|
||||
|
||||
var (
|
||||
errForkPRManualStartRequired = errors.New("fork pull request review requires a manual start")
|
||||
errOfficeTaskStartRequiresScheduler = errors.New(
|
||||
"office tasks must be started through Office; use StartTaskWithEnv with scheduler-injected credentials",
|
||||
)
|
||||
|
||||
@@ -178,6 +178,10 @@ const (
|
||||
// because the winner will (or already did) handle it.
|
||||
// Absent on ordinary (non-watcher) auto-start tasks, which launch normally.
|
||||
MetaKeyAutoStartClaimed = "auto_start_claimed"
|
||||
// MetaKeyForkPRRequiresManualStart blocks unattended launches of fork PR
|
||||
// review tasks. Their worktree setup scripts can read executor-profile secrets;
|
||||
// a user can still explicitly start the task after reviewing it.
|
||||
MetaKeyForkPRRequiresManualStart = "fork_pr_requires_manual_start"
|
||||
// MetaKeyInterruptedAt is set by reconciliation when a task's session lost
|
||||
// an active turn. Its presence makes the task DTO report `interrupted: true`
|
||||
// so task-list surfaces show the warning indicator. The orchestrator removes
|
||||
|
||||
@@ -4,6 +4,12 @@ import { useRegularMode } from "../../helpers/regular-mode";
|
||||
import { KanbanPage } from "../../pages/kanban-page";
|
||||
import { SessionPage } from "../../pages/session-page";
|
||||
import { createEmptyRemoteRepository } from "../../helpers/empty-remote-repository";
|
||||
import { waitForHttp } from "../../helpers/causal-waits";
|
||||
import {
|
||||
cleanupPRLinkForkLaunchFixture,
|
||||
createPRLinkForkLaunchFixture,
|
||||
expectForkPRLaunchState,
|
||||
} from "./pr-link-fork-launch-helpers";
|
||||
|
||||
// Exercises the regular task-create dialog (New Task in the sidebar); run with office off.
|
||||
useRegularMode();
|
||||
@@ -174,6 +180,72 @@ test.describe("Task creation from GitHub URL", () => {
|
||||
await expect(session.idleInput()).toBeVisible({ timeout: 15_000 });
|
||||
});
|
||||
|
||||
test("starts a target-attached fork PR from its URL", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
backend,
|
||||
}) => {
|
||||
test.setTimeout(120_000);
|
||||
const fixture = await createPRLinkForkLaunchFixture(
|
||||
apiClient,
|
||||
seedData.workspaceId,
|
||||
backend.tmpDir,
|
||||
);
|
||||
let taskId: string | undefined;
|
||||
|
||||
try {
|
||||
const { executors } = await apiClient.listExecutors();
|
||||
const worktreeExec = executors.find((executor) => executor.type === "worktree");
|
||||
if (!worktreeExec?.profiles?.[0]) {
|
||||
test.skip(true, "No worktree executor profile available");
|
||||
return;
|
||||
}
|
||||
|
||||
const taskTitle = `Fork PR launch ${fixture.repositoryName}`;
|
||||
const kanban = new KanbanPage(testPage);
|
||||
await kanban.goto();
|
||||
await kanban.createTaskButton.first().click();
|
||||
const dialog = testPage.getByTestId("create-task-dialog");
|
||||
await expect(dialog).toBeVisible();
|
||||
await openRemoteAndPasteURL(testPage, fixture.prURL);
|
||||
await expect(testPage.getByTestId("remote-branch-chip-trigger").first()).toContainText(
|
||||
fixture.headBranch,
|
||||
);
|
||||
await testPage.getByTestId("task-title-input").fill(taskTitle);
|
||||
await testPage.getByTestId("task-description-input").fill("/e2e:simple-message");
|
||||
|
||||
const startButton = testPage.getByTestId("submit-start-agent");
|
||||
await expect(startButton).toBeEnabled();
|
||||
await testPage.getByTestId("executor-profile-selector").click();
|
||||
await testPage.getByRole("option", { name: /Worktree/i }).click();
|
||||
const createdTaskResponse = waitForHttp(testPage, "POST", /\/api\/v1\/tasks$/);
|
||||
await startButton.click();
|
||||
const response = await createdTaskResponse;
|
||||
const responseBody = await response.text();
|
||||
expect(response.status(), responseBody).toBe(200);
|
||||
const created = JSON.parse(responseBody) as { id: string };
|
||||
taskId = created.id;
|
||||
const requestBody = response.request().postDataJSON() as {
|
||||
repositories?: Array<Record<string, unknown>>;
|
||||
};
|
||||
expect(requestBody.repositories?.[0]).not.toHaveProperty("remote_contribution");
|
||||
expect(requestBody.repositories?.[0]).not.toHaveProperty("comparison_target");
|
||||
|
||||
await expect(dialog).not.toBeVisible();
|
||||
await expect(testPage).toHaveURL(new RegExp(`/t/${taskId}$`));
|
||||
const session = new SessionPage(testPage);
|
||||
await session.waitForLoad();
|
||||
await expect(session.chat.getByText("simple mock response", { exact: false })).toBeVisible();
|
||||
await expect(session.idleInput()).toBeVisible();
|
||||
|
||||
await expectForkPRLaunchState(testPage, session, apiClient, fixture, taskId);
|
||||
await expect(session.prTopbarButton()).toContainText("#3879", { timeout: 15_000 });
|
||||
} finally {
|
||||
await cleanupPRLinkForkLaunchFixture(apiClient, fixture, taskId);
|
||||
}
|
||||
});
|
||||
|
||||
// Three tests previously asserted the top-level `github-url-error` testid
|
||||
// surfaced on invalid URL, nonexistent repo, and "clears on valid URL".
|
||||
// After Task 5/8 the URL input moved into a per-chip popover and no
|
||||
|
||||
@@ -1,6 +1,14 @@
|
||||
import { test, expect } from "../../fixtures/test-base";
|
||||
import type { Page } from "@playwright/test";
|
||||
import { MobileKanbanPage } from "../../pages/mobile-kanban-page";
|
||||
import { SessionPage } from "../../pages/session-page";
|
||||
import { waitForHttp } from "../../helpers/causal-waits";
|
||||
import { switchToTerminalPanel, waitForShellReady } from "../terminal/mobile-terminal-helpers";
|
||||
import {
|
||||
cleanupPRLinkForkLaunchFixture,
|
||||
createPRLinkForkLaunchFixture,
|
||||
expectForkPRLaunchState,
|
||||
} from "./pr-link-fork-launch-helpers";
|
||||
|
||||
function expectedRemoteTitle(title: string): string {
|
||||
const characters = Array.from(title);
|
||||
@@ -96,6 +104,80 @@ test.describe("Create task Remote repo picker on mobile", () => {
|
||||
await expectNoDocumentHorizontalOverflow(testPage);
|
||||
});
|
||||
|
||||
test("starts a target-attached fork PR from its URL", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
backend,
|
||||
}) => {
|
||||
test.setTimeout(120_000);
|
||||
const fixture = await createPRLinkForkLaunchFixture(
|
||||
apiClient,
|
||||
seedData.workspaceId,
|
||||
backend.tmpDir,
|
||||
);
|
||||
let taskId: string | undefined;
|
||||
|
||||
try {
|
||||
const { executors } = await apiClient.listExecutors();
|
||||
const worktreeExec = executors.find((executor) => executor.type === "worktree");
|
||||
if (!worktreeExec?.profiles?.[0]) {
|
||||
test.skip(true, "No worktree executor profile available");
|
||||
return;
|
||||
}
|
||||
|
||||
const taskTitle = `Phone fork PR ${fixture.repositoryName}`;
|
||||
const mobile = new MobileKanbanPage(testPage);
|
||||
await mobile.goto();
|
||||
await mobile.mobileFab.tap();
|
||||
const dialog = testPage.getByTestId("create-task-dialog");
|
||||
await expect(dialog).toBeVisible();
|
||||
await testPage.getByTestId("source-mode-remote").tap();
|
||||
await testPage.getByTestId("remote-repo-chip-trigger").first().tap();
|
||||
const urlInput = testPage.getByTestId("remote-repo-input");
|
||||
await expect(urlInput).toBeVisible();
|
||||
await urlInput.fill(fixture.prURL);
|
||||
await urlInput.press("Enter");
|
||||
await expect(testPage.getByTestId("remote-branch-chip-trigger").first()).toContainText(
|
||||
fixture.headBranch,
|
||||
);
|
||||
await testPage.getByTestId("task-title-input").fill(taskTitle);
|
||||
await testPage.getByTestId("task-description-input").fill("/e2e:simple-message");
|
||||
|
||||
const startButton = testPage.getByTestId("submit-start-agent");
|
||||
await expect(startButton).toBeEnabled();
|
||||
await testPage.getByTestId("executor-profile-selector").tap();
|
||||
await testPage.getByRole("option", { name: /Worktree/i }).tap();
|
||||
const createdTaskResponse = waitForHttp(testPage, "POST", /\/api\/v1\/tasks$/);
|
||||
await startButton.tap();
|
||||
const response = await createdTaskResponse;
|
||||
const responseBody = await response.text();
|
||||
expect(response.status(), responseBody).toBe(200);
|
||||
const created = JSON.parse(responseBody) as { id: string };
|
||||
taskId = created.id;
|
||||
const requestBody = response.request().postDataJSON() as {
|
||||
repositories?: Array<Record<string, unknown>>;
|
||||
};
|
||||
expect(requestBody.repositories?.[0]).not.toHaveProperty("remote_contribution");
|
||||
expect(requestBody.repositories?.[0]).not.toHaveProperty("comparison_target");
|
||||
|
||||
await expect(dialog).not.toBeVisible();
|
||||
await expect(mobile.taskCard(taskId)).toBeVisible({ timeout: 15_000 });
|
||||
await mobile.taskCard(taskId).tap();
|
||||
await expect(testPage).toHaveURL(new RegExp(`/t/${taskId}$`));
|
||||
const session = new SessionPage(testPage);
|
||||
await session.waitForLoad();
|
||||
await expect(session.chat.getByText("simple mock response", { exact: false })).toBeVisible();
|
||||
await expect(session.idleInput()).toBeVisible();
|
||||
|
||||
await switchToTerminalPanel(testPage);
|
||||
await waitForShellReady(testPage);
|
||||
await expectForkPRLaunchState(testPage, session, apiClient, fixture, taskId);
|
||||
} finally {
|
||||
await cleanupPRLinkForkLaunchFixture(apiClient, fixture, taskId);
|
||||
}
|
||||
});
|
||||
|
||||
test("keeps a failed URL row touch-usable and retries its branch resolution", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
|
||||
@@ -0,0 +1,211 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { expect, type Page } from "@playwright/test";
|
||||
import {
|
||||
createEmptyRemoteRepository,
|
||||
removeTestRepository,
|
||||
} from "../../helpers/empty-remote-repository";
|
||||
import { GitHelper } from "../../helpers/git-helper";
|
||||
import type { ApiClient } from "../../helpers/api-client";
|
||||
import type { SessionPage } from "../../pages/session-page";
|
||||
|
||||
const PR_NUMBER = 3879;
|
||||
const HEAD_BRANCH = "feature/fork-pr-launch";
|
||||
|
||||
export async function expectTerminalCommit(page: Page, commitOID: string): Promise<void> {
|
||||
const readUnwrappedTerminal = async () =>
|
||||
page
|
||||
.locator('[data-testid="terminal-panel"]:visible')
|
||||
.first()
|
||||
.locator(".xterm")
|
||||
.evaluate((xtermElement) => {
|
||||
const container = xtermElement.parentElement as HTMLElement & {
|
||||
__xtermReadBuffer?: () => string;
|
||||
};
|
||||
return (container.__xtermReadBuffer?.() ?? "").replace(/\s+/gu, "");
|
||||
});
|
||||
|
||||
await expect
|
||||
.poll(readUnwrappedTerminal, {
|
||||
timeout: 10_000,
|
||||
message: `Expected terminal to contain commit ${commitOID} without line wrapping`,
|
||||
})
|
||||
.toContain(commitOID);
|
||||
}
|
||||
|
||||
export type PRLinkForkLaunchFixture = {
|
||||
repositoryId: string;
|
||||
repositoryName: string;
|
||||
upstreamOwner: string;
|
||||
upstreamRepository: string;
|
||||
forkOwner: string;
|
||||
forkRepository: string;
|
||||
prURL: string;
|
||||
headBranch: string;
|
||||
headOID: string;
|
||||
targetOID: string;
|
||||
forkCleanup: () => void;
|
||||
upstreamCleanup: () => void;
|
||||
};
|
||||
|
||||
export async function expectForkPRLaunchState(
|
||||
page: Page,
|
||||
session: SessionPage,
|
||||
apiClient: ApiClient,
|
||||
fixture: PRLinkForkLaunchFixture,
|
||||
taskId: string,
|
||||
): Promise<void> {
|
||||
await expect(session.terminal).toBeVisible();
|
||||
await session.typeInTerminal("git branch --show-current");
|
||||
await session.expectTerminalHasText(fixture.headBranch);
|
||||
await session.typeInTerminal("git rev-parse HEAD");
|
||||
await expectTerminalCommit(page, fixture.headOID);
|
||||
await session.typeInTerminal("git rev-parse refs/remotes/origin/main");
|
||||
await expectTerminalCommit(page, fixture.targetOID);
|
||||
|
||||
await page.reload();
|
||||
await session.waitForLoad();
|
||||
const task = await apiClient.getTask(taskId);
|
||||
expect(task.repositories?.[0]?.checkout_branch).toBe(fixture.headBranch);
|
||||
expect(task.repositories?.[0]?.base_branch).toBe("main");
|
||||
await expect
|
||||
.poll(async () => (await apiClient.getTask(taskId)).status_summary?.pull_request?.number, {
|
||||
timeout: 15_000,
|
||||
message: "waiting for the persisted PR summary to reach the task list",
|
||||
})
|
||||
.toBe(PR_NUMBER);
|
||||
await expect
|
||||
.poll(async () =>
|
||||
(await apiClient.listTaskPRs(taskId)).map((pr) => ({
|
||||
owner: pr.owner,
|
||||
repo: pr.repo,
|
||||
pr_number: pr.pr_number,
|
||||
head_branch: pr.head_branch,
|
||||
})),
|
||||
)
|
||||
.toEqual([
|
||||
{
|
||||
owner: fixture.upstreamOwner,
|
||||
repo: fixture.upstreamRepository,
|
||||
pr_number: PR_NUMBER,
|
||||
head_branch: fixture.headBranch,
|
||||
},
|
||||
]);
|
||||
}
|
||||
|
||||
export async function createPRLinkForkLaunchFixture(
|
||||
apiClient: ApiClient,
|
||||
workspaceId: string,
|
||||
tmpDir: string,
|
||||
): Promise<PRLinkForkLaunchFixture> {
|
||||
const suffix = randomUUID().replaceAll("-", "").slice(0, 10);
|
||||
const upstreamOwner = `e2e-upstream-${suffix}`;
|
||||
const upstreamRepository = `pr-link-upstream-${suffix}`;
|
||||
const forkOwner = `e2e-fork-${suffix}`;
|
||||
const forkRepository = `pr-link-fork-${suffix}`;
|
||||
const upstream = createEmptyRemoteRepository(tmpDir, `pr-link-upstream-${suffix}`);
|
||||
const fork = createEmptyRemoteRepository(tmpDir, `pr-link-fork-${suffix}`);
|
||||
let repositoryId = "";
|
||||
|
||||
try {
|
||||
const upstreamGit = new GitHelper(upstream.localPath, upstream.gitEnv);
|
||||
upstreamGit.createFile("base.txt", "upstream target main\n");
|
||||
upstreamGit.stageAll();
|
||||
const targetOID = upstreamGit.commit("upstream target main");
|
||||
upstreamGit.exec("git push origin main");
|
||||
|
||||
const forkGit = new GitHelper(fork.localPath, fork.gitEnv);
|
||||
forkGit.createFile("base.txt", "fork main\n");
|
||||
forkGit.stageAll();
|
||||
forkGit.commit("fork main");
|
||||
forkGit.exec("git push origin main");
|
||||
forkGit.exec(`git checkout -b ${HEAD_BRANCH}`);
|
||||
forkGit.createFile("pr.txt", "fork pull request head\n");
|
||||
forkGit.stageAll();
|
||||
const headOID = forkGit.commit("fork pull request head");
|
||||
forkGit.exec(`git push origin ${HEAD_BRANCH}`);
|
||||
forkGit.exec(`git push "${upstream.remoteURL}" HEAD:refs/pull/${PR_NUMBER}/head`);
|
||||
|
||||
repositoryId = (
|
||||
await apiClient.createRepository(workspaceId, upstream.localPath, "main", {
|
||||
name: `${upstreamOwner}/${upstreamRepository}`,
|
||||
provider: "github",
|
||||
provider_owner: upstreamOwner,
|
||||
provider_name: upstreamRepository,
|
||||
pull_before_worktree: false,
|
||||
})
|
||||
).id;
|
||||
await apiClient.mockGitHubAddBranches(upstreamOwner, upstreamRepository, [
|
||||
{ name: "main" },
|
||||
{ name: HEAD_BRANCH },
|
||||
]);
|
||||
await apiClient.mockGitHubAddPRs([
|
||||
{
|
||||
number: PR_NUMBER,
|
||||
title: "Fork PR task launch",
|
||||
state: "open",
|
||||
head_branch: HEAD_BRANCH,
|
||||
head_sha: headOID,
|
||||
head_repo_id: 387901,
|
||||
head_repo_owner: forkOwner,
|
||||
head_repo_name: forkRepository,
|
||||
head_repo_clone_url: `https://github.com/${forkOwner}/${forkRepository}.git`,
|
||||
base_branch: "main",
|
||||
base_repo_id: 387902,
|
||||
base_repo_owner: upstreamOwner,
|
||||
base_repo_name: upstreamRepository,
|
||||
base_default_branch: "main",
|
||||
author_login: "e2e-fork-author",
|
||||
repo_owner: upstreamOwner,
|
||||
repo_name: upstreamRepository,
|
||||
},
|
||||
]);
|
||||
|
||||
return {
|
||||
repositoryId,
|
||||
repositoryName: `${upstreamOwner}/${upstreamRepository}`,
|
||||
upstreamOwner,
|
||||
upstreamRepository,
|
||||
forkOwner,
|
||||
forkRepository,
|
||||
prURL: `https://github.com/${upstreamOwner}/${upstreamRepository}/pull/${PR_NUMBER}`,
|
||||
headBranch: HEAD_BRANCH,
|
||||
headOID,
|
||||
targetOID,
|
||||
forkCleanup: fork.cleanup,
|
||||
upstreamCleanup: upstream.cleanup,
|
||||
};
|
||||
} catch (error) {
|
||||
if (repositoryId) await removeTestRepository(apiClient, repositoryId).catch(() => undefined);
|
||||
fork.cleanup();
|
||||
upstream.cleanup();
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
export async function cleanupPRLinkForkLaunchFixture(
|
||||
apiClient: ApiClient,
|
||||
fixture: PRLinkForkLaunchFixture,
|
||||
taskId?: string,
|
||||
): Promise<void> {
|
||||
let cleanupError: unknown;
|
||||
if (taskId) {
|
||||
try {
|
||||
await apiClient.deleteTask(taskId);
|
||||
} catch (error) {
|
||||
cleanupError = error;
|
||||
}
|
||||
}
|
||||
try {
|
||||
await removeTestRepository(apiClient, fixture.repositoryId);
|
||||
} catch (error) {
|
||||
cleanupError ??= error;
|
||||
}
|
||||
for (const cleanup of [fixture.forkCleanup, fixture.upstreamCleanup]) {
|
||||
try {
|
||||
cleanup();
|
||||
} catch (error) {
|
||||
cleanupError ??= error;
|
||||
}
|
||||
}
|
||||
if (cleanupError) throw cleanupError;
|
||||
}
|
||||
Reference in New Issue
Block a user