diff --git a/apps/backend/cmd/kandev/adapters.go b/apps/backend/cmd/kandev/adapters.go index 78071c0317..39c1f48ef5 100644 --- a/apps/backend/cmd/kandev/adapters.go +++ b/apps/backend/cmd/kandev/adapters.go @@ -103,6 +103,7 @@ func (a *lifecycleAdapter) LaunchAgent(ctx context.Context, req *executor.Launch BaseBranch: req.BaseBranch, DefaultBranch: req.DefaultBranch, CheckoutBranch: req.CheckoutBranch, + PRNumber: req.PRNumber, WorktreeBranchPrefix: req.WorktreeBranchPrefix, PullBeforeWorktree: req.PullBeforeWorktree, // Task directory mode @@ -133,6 +134,7 @@ func (a *lifecycleAdapter) LaunchAgent(ctx context.Context, req *executor.Launch BaseBranch: r.BaseBranch, DefaultBranch: r.DefaultBranch, CheckoutBranch: r.CheckoutBranch, + PRNumber: r.PRNumber, WorktreeID: r.WorktreeID, WorktreeBranchPrefix: r.WorktreeBranchPrefix, PullBeforeWorktree: r.PullBeforeWorktree, diff --git a/apps/backend/cmd/kandev/orchestrator.go b/apps/backend/cmd/kandev/orchestrator.go index b9e1d9a88f..695c1f9a51 100644 --- a/apps/backend/cmd/kandev/orchestrator.go +++ b/apps/backend/cmd/kandev/orchestrator.go @@ -245,6 +245,7 @@ func (a *reviewTaskCreatorAdapter) CreateReviewTask(ctx context.Context, req *or RepositoryID: r.RepositoryID, BaseBranch: r.BaseBranch, CheckoutBranch: r.CheckoutBranch, + PRNumber: r.PRNumber, }) } return a.svc.CreateTask(ctx, &taskservice.CreateTaskRequest{ diff --git a/apps/backend/internal/agent/runtime/lifecycle/env_preparer.go b/apps/backend/internal/agent/runtime/lifecycle/env_preparer.go index eb6f03b3bc..c8a09b0e06 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/env_preparer.go +++ b/apps/backend/internal/agent/runtime/lifecycle/env_preparer.go @@ -32,6 +32,7 @@ type RepoPrepareSpec struct { BaseBranch 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. WorktreeID string WorktreeBranch string WorktreeBranchPrefix string @@ -55,6 +56,7 @@ type EnvPrepareRequest struct { BaseBranch 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. WorktreeID string WorktreeBranch string @@ -91,6 +93,7 @@ func (r *EnvPrepareRequest) RepoSpecs() []RepoPrepareSpec { BaseBranch: r.BaseBranch, DefaultBranch: r.DefaultBranch, CheckoutBranch: r.CheckoutBranch, + PRNumber: r.PRNumber, WorktreeID: r.WorktreeID, WorktreeBranch: r.WorktreeBranch, WorktreeBranchPrefix: r.WorktreeBranchPrefix, 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 b9c8cfd4b8..d24e45c1e1 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree.go +++ b/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree.go @@ -174,6 +174,7 @@ func (p *WorktreePreparer) createWorktreeWithSync( BaseBranch: req.BaseBranch, FallbackBaseBranch: req.DefaultBranch, CheckoutBranch: req.CheckoutBranch, + PRNumber: req.PRNumber, WorktreeBranchPrefix: req.WorktreeBranchPrefix, PullBeforeWorktree: req.PullBeforeWorktree, WorktreeID: req.WorktreeID, @@ -380,6 +381,7 @@ func (p *WorktreePreparer) prepareOneRepo( subReq.BaseBranch = spec.BaseBranch subReq.DefaultBranch = spec.DefaultBranch subReq.CheckoutBranch = spec.CheckoutBranch + subReq.PRNumber = spec.PRNumber subReq.WorktreeID = spec.WorktreeID subReq.WorktreeBranchPrefix = spec.WorktreeBranchPrefix subReq.PullBeforeWorktree = spec.PullBeforeWorktree diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go b/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go index 7a5dabacec..9dd0604100 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go @@ -561,6 +561,7 @@ func buildEnvPrepareRequest(req *LaunchRequest, workspacePath string, execName e BaseBranch: req.BaseBranch, DefaultBranch: req.DefaultBranch, CheckoutBranch: req.CheckoutBranch, + PRNumber: req.PRNumber, WorktreeBranch: getMetadataString(req.Metadata, MetadataKeyWorktreeBranch), WorktreeBranchPrefix: req.WorktreeBranchPrefix, PullBeforeWorktree: req.PullBeforeWorktree, @@ -585,6 +586,7 @@ func buildEnvPrepareRequest(req *LaunchRequest, workspacePath string, execName e BaseBranch: r.BaseBranch, DefaultBranch: r.DefaultBranch, CheckoutBranch: r.CheckoutBranch, + PRNumber: r.PRNumber, WorktreeID: r.WorktreeID, WorktreeBranchPrefix: r.WorktreeBranchPrefix, PullBeforeWorktree: r.PullBeforeWorktree, diff --git a/apps/backend/internal/agent/runtime/lifecycle/types.go b/apps/backend/internal/agent/runtime/lifecycle/types.go index b0e329726b..fa234989a9 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/types.go +++ b/apps/backend/internal/agent/runtime/lifecycle/types.go @@ -301,6 +301,7 @@ type RepoLaunchSpec struct { BaseBranch 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. WorktreeID string // Existing worktree ID to reuse (skip creation if set) WorktreeBranchPrefix string PullBeforeWorktree bool @@ -368,6 +369,7 @@ type LaunchRequest struct { BaseBranch string // Base branch for the worktree (e.g., "main") 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. WorktreeBranchPrefix string // Branch prefix for worktree branches PullBeforeWorktree bool // Whether to pull from remote before creating the worktree @@ -400,6 +402,7 @@ func (r *LaunchRequest) RepoSpecs() []RepoLaunchSpec { BaseBranch: r.BaseBranch, DefaultBranch: r.DefaultBranch, CheckoutBranch: r.CheckoutBranch, + PRNumber: r.PRNumber, WorktreeID: r.WorktreeID, WorktreeBranchPrefix: r.WorktreeBranchPrefix, PullBeforeWorktree: r.PullBeforeWorktree, diff --git a/apps/backend/internal/orchestrator/event_handlers_github.go b/apps/backend/internal/orchestrator/event_handlers_github.go index 5b982a4e15..8eb528ab3b 100644 --- a/apps/backend/internal/orchestrator/event_handlers_github.go +++ b/apps/backend/internal/orchestrator/event_handlers_github.go @@ -98,6 +98,7 @@ type ReviewTaskRepository struct { RepositoryID string BaseBranch string CheckoutBranch string + PRNumber int // GitHub PR number; carried so worktree creation can use refs/pull//head for fork PRs. } // SetGitHubService sets the GitHub service for PR auto-detection. @@ -367,7 +368,7 @@ func (s *Service) resolveReviewRepository(ctx context.Context, workspaceID strin zap.String("base_branch", baseBranch)) // BaseBranch = repo default branch (e.g. "main") for worktree creation. // CheckoutBranch = PR head branch to fetch and checkout after worktree is created. - return []ReviewTaskRepository{{RepositoryID: repoID, BaseBranch: baseBranch, CheckoutBranch: pr.HeadBranch}} + return []ReviewTaskRepository{{RepositoryID: repoID, BaseBranch: baseBranch, CheckoutBranch: pr.HeadBranch, PRNumber: pr.Number}} } // detectPushAndAssociatePR checks if a push happened and looks for a PR on diff --git a/apps/backend/internal/orchestrator/executor/executor.go b/apps/backend/internal/orchestrator/executor/executor.go index 65cd1b11d8..077876f702 100644 --- a/apps/backend/internal/orchestrator/executor/executor.go +++ b/apps/backend/internal/orchestrator/executor/executor.go @@ -280,6 +280,7 @@ type LaunchAgentRequest struct { BaseBranch string // Base branch for the worktree (e.g., "main") 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. WorktreeBranchPrefix string // Branch prefix for worktree branches PullBeforeWorktree bool // Whether to pull from remote before creating the worktree @@ -311,6 +312,7 @@ type RepoSpec struct { BaseBranch 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. WorktreeID string WorktreeBranchPrefix string PullBeforeWorktree bool diff --git a/apps/backend/internal/orchestrator/executor/executor_execute.go b/apps/backend/internal/orchestrator/executor/executor_execute.go index cc0fa956d5..7dcd020537 100644 --- a/apps/backend/internal/orchestrator/executor/executor_execute.go +++ b/apps/backend/internal/orchestrator/executor/executor_execute.go @@ -697,6 +697,7 @@ func buildRepoSpecs(allRepos []*repoInfo) []RepoSpec { RepositoryPath: info.RepositoryPath, BaseBranch: info.BaseBranch, CheckoutBranch: info.CheckoutBranch, + PRNumber: info.PRNumber, WorktreeBranchPrefix: info.WorktreeBranchPrefix, PullBeforeWorktree: info.PullBeforeWorktree, } @@ -726,6 +727,7 @@ func (e *Executor) applyRepositoryConfig(req *LaunchAgentRequest, task *v1.Task, req.RepositoryPath = repoInfo.RepositoryPath req.BaseBranch = repoInfo.BaseBranch req.CheckoutBranch = repoInfo.CheckoutBranch + req.PRNumber = repoInfo.PRNumber req.WorktreeBranchPrefix = repoInfo.WorktreeBranchPrefix req.PullBeforeWorktree = repoInfo.PullBeforeWorktree if repoInfo.Repository != nil { diff --git a/apps/backend/internal/orchestrator/executor/executor_resume.go b/apps/backend/internal/orchestrator/executor/executor_resume.go index 021d08affe..fffcf358c4 100644 --- a/apps/backend/internal/orchestrator/executor/executor_resume.go +++ b/apps/backend/internal/orchestrator/executor/executor_resume.go @@ -38,6 +38,7 @@ type repoInfo struct { RepositoryPath string BaseBranch string CheckoutBranch string + PRNumber int // GitHub PR number when CheckoutBranch is a PR head; sourced from task_repositories.metadata["pr_number"]. WorktreeBranchPrefix string PullBeforeWorktree bool Repository *models.Repository @@ -92,6 +93,7 @@ func (e *Executor) resolveTaskRepoInfo(ctx context.Context, tr *models.TaskRepos RepositoryID: tr.RepositoryID, BaseBranch: tr.BaseBranch, CheckoutBranch: tr.CheckoutBranch, + PRNumber: prNumberFromMetadata(tr.Metadata), } if info.RepositoryID == "" { return info, nil @@ -714,6 +716,7 @@ func (e *Executor) applyResumeRepoConfig(ctx context.Context, task *v1.Task, ses primaryTaskRepo, _ := e.repo.GetPrimaryTaskRepository(ctx, task.ID) if primaryTaskRepo != nil && primaryTaskRepo.CheckoutBranch != "" { req.CheckoutBranch = primaryTaskRepo.CheckoutBranch + req.PRNumber = prNumberFromMetadata(primaryTaskRepo.Metadata) } req.WorktreeBranchPrefix = repository.WorktreeBranchPrefix req.PullBeforeWorktree = repository.PullBeforeWorktree @@ -761,6 +764,35 @@ func (e *Executor) persistResumeState(ctx context.Context, taskID string, sessio } } +// prNumberFromMetadata extracts a GitHub PR number from a task_repository's +// metadata bag. Stored as JSON, so on retrieval the value can decode as either +// float64 (default for json.Unmarshal into interface{}) or int. Returns 0 when +// the key is absent, malformed, or non-positive. +func prNumberFromMetadata(metadata map[string]interface{}) int { + if metadata == nil { + return 0 + } + raw, ok := metadata["pr_number"] + if !ok { + return 0 + } + switch v := raw.(type) { + case float64: + if v > 0 { + return int(v) + } + case int: + if v > 0 { + return v + } + case int64: + if v > 0 { + return int(v) + } + } + return 0 +} + // startAgentProcessOnResume starts the agent process asynchronously after a session resume. // Task state is managed by workflow triggers and stream handlers elsewhere; this callback // just logs successful process start. diff --git a/apps/backend/internal/orchestrator/executor/pr_number_test.go b/apps/backend/internal/orchestrator/executor/pr_number_test.go new file mode 100644 index 0000000000..fedcf3c067 --- /dev/null +++ b/apps/backend/internal/orchestrator/executor/pr_number_test.go @@ -0,0 +1,32 @@ +package executor + +import "testing" + +// TestPRNumberFromMetadata documents the type-tolerance the helper needs: +// task_repository metadata is round-tripped through JSON, so an integer +// stored on write surfaces as float64 on read. Negative or zero values are +// treated as "no PR" so callers don't accidentally enable the PR refspec +// path with garbage data. +func TestPRNumberFromMetadata(t *testing.T) { + cases := []struct { + name string + md map[string]interface{} + want int + }{ + {name: "nil map", md: nil, want: 0}, + {name: "missing key", md: map[string]interface{}{}, want: 0}, + {name: "float64 (post-json)", md: map[string]interface{}{"pr_number": float64(974)}, want: 974}, + {name: "int (pre-json roundtrip)", md: map[string]interface{}{"pr_number": 42}, want: 42}, + {name: "int64", md: map[string]interface{}{"pr_number": int64(7)}, want: 7}, + {name: "zero is rejected", md: map[string]interface{}{"pr_number": float64(0)}, want: 0}, + {name: "negative is rejected", md: map[string]interface{}{"pr_number": float64(-1)}, want: 0}, + {name: "wrong type ignored", md: map[string]interface{}{"pr_number": "974"}, want: 0}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := prNumberFromMetadata(tc.md); got != tc.want { + t.Fatalf("prNumberFromMetadata(%v) = %d, want %d", tc.md, got, tc.want) + } + }) + } +} diff --git a/apps/backend/internal/task/dto/requests.go b/apps/backend/internal/task/dto/requests.go index 1951654ff7..628d40b321 100644 --- a/apps/backend/internal/task/dto/requests.go +++ b/apps/backend/internal/task/dto/requests.go @@ -248,6 +248,7 @@ type TaskRepositoryInput struct { RepositoryID string BaseBranch string CheckoutBranch string + PRNumber int // GitHub PR number when CheckoutBranch is a PR head; persisted into task_repositories.metadata["pr_number"]. LocalPath string Name string DefaultBranch string diff --git a/apps/backend/internal/task/handlers/task_handlers.go b/apps/backend/internal/task/handlers/task_handlers.go index 764f046be4..ff54f902fc 100644 --- a/apps/backend/internal/task/handlers/task_handlers.go +++ b/apps/backend/internal/task/handlers/task_handlers.go @@ -140,6 +140,7 @@ func convertToServiceRepos(repos []dto.TaskRepositoryInput) []service.TaskReposi RepositoryID: r.RepositoryID, BaseBranch: r.BaseBranch, CheckoutBranch: r.CheckoutBranch, + PRNumber: r.PRNumber, LocalPath: r.LocalPath, Name: r.Name, DefaultBranch: r.DefaultBranch, diff --git a/apps/backend/internal/task/service/pr_number_test.go b/apps/backend/internal/task/service/pr_number_test.go new file mode 100644 index 0000000000..df74d111ff --- /dev/null +++ b/apps/backend/internal/task/service/pr_number_test.go @@ -0,0 +1,63 @@ +package service + +import "testing" + +// TestResolvePRNumber covers every branch of the helper that surfaces a PR +// number for downstream worktree creation. Explicit PRNumber always wins; +// GitHubURL is parsed only when it carries a /pull/ path. +func TestResolvePRNumber(t *testing.T) { + cases := []struct { + name string + input TaskRepositoryInput + want int + }{ + { + name: "explicit pr number wins", + input: TaskRepositoryInput{PRNumber: 42, GitHubURL: "https://github.com/owner/repo/pull/99"}, + want: 42, + }, + { + name: "parses pr number from pull URL", + input: TaskRepositoryInput{GitHubURL: "https://github.com/kdlbs/kandev/pull/974"}, + want: 974, + }, + { + name: "parses with trailing slash", + input: TaskRepositoryInput{GitHubURL: "https://github.com/owner/repo/pull/123/"}, + want: 123, + }, + { + name: "parses with query string", + input: TaskRepositoryInput{GitHubURL: "https://github.com/owner/repo/pull/7?diff=unified"}, + want: 7, + }, + { + name: "parses with fragment", + input: TaskRepositoryInput{GitHubURL: "https://github.com/owner/repo/pull/8#discussion_r1"}, + want: 8, + }, + { + name: "non-pr github URL returns zero", + input: TaskRepositoryInput{GitHubURL: "https://github.com/owner/repo"}, + want: 0, + }, + { + name: "empty input returns zero", + input: TaskRepositoryInput{}, + want: 0, + }, + { + name: "malformed pull segment returns zero", + input: TaskRepositoryInput{GitHubURL: "https://github.com/owner/repo/pull/abc"}, + want: 0, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := resolvePRNumber(tc.input); got != tc.want { + t.Fatalf("resolvePRNumber(%+v) = %d, want %d", tc.input, got, tc.want) + } + }) + } +} diff --git a/apps/backend/internal/task/service/service_requests.go b/apps/backend/internal/task/service/service_requests.go index e9a43ba1cf..ba219447f8 100644 --- a/apps/backend/internal/task/service/service_requests.go +++ b/apps/backend/internal/task/service/service_requests.go @@ -13,6 +13,7 @@ type TaskRepositoryInput struct { RepositoryID string `json:"repository_id"` BaseBranch string `json:"base_branch"` CheckoutBranch string `json:"checkout_branch,omitempty"` + PRNumber int `json:"pr_number,omitempty"` // GitHub PR number when CheckoutBranch is a PR head; persisted into task_repositories.metadata["pr_number"]. LocalPath string `json:"local_path,omitempty"` Name string `json:"name,omitempty"` DefaultBranch string `json:"default_branch,omitempty"` diff --git a/apps/backend/internal/task/service/service_tasks.go b/apps/backend/internal/task/service/service_tasks.go index c0b64936ff..52257a153f 100644 --- a/apps/backend/internal/task/service/service_tasks.go +++ b/apps/backend/internal/task/service/service_tasks.go @@ -6,6 +6,7 @@ import ( "net/url" "os" "path/filepath" + "strconv" "strings" "time" @@ -247,13 +248,17 @@ func (s *Service) createTaskRepositories(ctx context.Context, taskID, workspaceI return fmt.Errorf("repository %q listed more than once for the task", repositoryID) } seen[repositoryID] = true + metadata := make(map[string]interface{}) + if prNum := resolvePRNumber(repoInput); prNum > 0 { + metadata["pr_number"] = prNum + } taskRepo := &models.TaskRepository{ TaskID: taskID, RepositoryID: repositoryID, BaseBranch: baseBranch, CheckoutBranch: repoInput.CheckoutBranch, Position: i, - Metadata: make(map[string]interface{}), + Metadata: metadata, } if err := s.taskRepos.CreateTaskRepository(ctx, taskRepo); err != nil { s.logger.Error("failed to create task repository", zap.Error(err)) @@ -396,6 +401,34 @@ func parseGitHubRepoURL(rawURL string) (owner, name string, err error) { return parts[0], parts[1], nil } +// resolvePRNumber returns the GitHub PR number for a repository input. Prefers +// the explicit PRNumber field; falls back to parsing a /pull/ path out of +// GitHubURL when present. Returns 0 when no PR is identified. +// +// The PR number is needed at worktree-creation time so fork PRs (whose head +// branch only exists on the contributor's fork) can be materialized via the +// refs/pull//head refspec on the base repo instead of a branch-name fetch +// that would 404 against origin. +func resolvePRNumber(input TaskRepositoryInput) int { + if input.PRNumber > 0 { + return input.PRNumber + } + rawURL := strings.TrimSpace(input.GitHubURL) + idx := strings.Index(rawURL, "/pull/") + if idx < 0 { + return 0 + } + numStr := rawURL[idx+len("/pull/"):] + if i := strings.IndexAny(numStr, "/?#"); i >= 0 { + numStr = numStr[:i] + } + n, err := strconv.Atoi(numStr) + if err != nil || n <= 0 { + return 0 + } + return n +} + // ReplaceTaskRepositories deletes all existing task-repository associations // and recreates them. Exported for callers that mutate repository inputs // (e.g. the fresh-branch flow rewriting BaseBranch) after CreateTask has diff --git a/apps/backend/internal/worktree/manager.go b/apps/backend/internal/worktree/manager.go index fb78ea7224..25159bc333 100644 --- a/apps/backend/internal/worktree/manager.go +++ b/apps/backend/internal/worktree/manager.go @@ -370,7 +370,7 @@ func (m *Manager) createInTaskDir(ctx context.Context, req CreateRequest, baseRe var fetchResult *FetchBranchResult if req.CheckoutBranch != "" { - fetchResult, err = m.fetchBranchToLocal(ctx, req.RepositoryPath, req.CheckoutBranch) + fetchResult, err = m.fetchBranchToLocal(ctx, req.RepositoryPath, req.CheckoutBranch, req.PRNumber) if err != nil { return nil, err } @@ -473,16 +473,26 @@ type FetchBranchResult struct { // fails (no remote, auth issue, offline), it falls back to the local branch. // Returns a FetchBranchResult with warning info and an error if the branch // doesn't exist anywhere. -func (m *Manager) fetchBranchToLocal(ctx context.Context, repoPath, branch string) (*FetchBranchResult, error) { +// +// When prNumber > 0, the fetch uses the refs/pull//head refspec instead of +// fetching the branch by name. GitHub mirrors every PR head under that ref on +// the base repo, so this is the only way to materialize a fork PR's head +// without adding the fork as a remote. +func (m *Manager) fetchBranchToLocal(ctx context.Context, repoPath, branch string, prNumber int) (*FetchBranchResult, error) { m.logger.Info("syncing checkout branch", zap.String("branch", branch), + zap.Int("pr_number", prNumber), zap.String("repo_path", repoPath)) // Try to fetch from origin to get the latest version. fetchCtx, cancelFetch := context.WithTimeout(ctx, m.fetchTimeout) defer cancelFetch() - fetchCmd := m.newNonInteractiveGitCmd(fetchCtx, repoPath, "fetch", gitNoTags, "origin", branch+":"+branch) + refspec := branch + ":" + branch + if prNumber > 0 { + refspec = fmt.Sprintf("pull/%d/head:%s", prNumber, branch) + } + fetchCmd := m.newNonInteractiveGitCmd(fetchCtx, repoPath, "fetch", gitNoTags, "origin", refspec) if output, err := fetchCmd.CombinedOutput(); err != nil { outputStr := string(output) @@ -490,10 +500,22 @@ func (m *Manager) fetchBranchToLocal(ctx context.Context, repoPath, branch strin // the local ref. Retry by fetching only the remote-tracking ref (origin/branch), // which is always safe regardless of worktree state. if isFetchRefusedCheckedOut(outputStr) { - retryCmd := m.newNonInteractiveGitCmd(fetchCtx, repoPath, "fetch", gitNoTags, "origin", branch) + retryRef := branch + if prNumber > 0 { + retryRef = fmt.Sprintf("pull/%d/head", prNumber) + } + retryCmd := m.newNonInteractiveGitCmd(fetchCtx, repoPath, "fetch", gitNoTags, "origin", retryRef) if _, retryErr := retryCmd.CombinedOutput(); retryErr == nil { m.logger.Info("fetched via remote-tracking ref (branch checked out elsewhere)", zap.String("branch", branch)) + if prNumber > 0 { + // Fork PRs have no origin/ ref — the bare + // pull//head retry only updates FETCH_HEAD, so the local + // `branch` (already populated by the prior worktree) is the + // only valid start point. Empty StartPoint signals the + // caller to fall back to req.CheckoutBranch. + return &FetchBranchResult{}, nil + } return &FetchBranchResult{StartPoint: "origin/" + branch}, nil } } diff --git a/apps/backend/internal/worktree/manager_pr_refspec_test.go b/apps/backend/internal/worktree/manager_pr_refspec_test.go new file mode 100644 index 0000000000..845cdb7080 --- /dev/null +++ b/apps/backend/internal/worktree/manager_pr_refspec_test.go @@ -0,0 +1,231 @@ +package worktree + +import ( + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// TestFetchBranchToLocal_PRNumberUsesPullRefspec verifies that when a PR +// number is supplied, the manager fetches refs/pull//head into the local +// branch. This is the fork-PR path: the head branch does not exist on origin +// by name (only as a pull//head ref), so the previous branch-name fetch +// would fail with "couldn't find remote ref". +func TestFetchBranchToLocal_PRNumberUsesPullRefspec(t *testing.T) { + repoPath, prHeadSHA := initGitRepoWithPullRef(t, 974, "feature/enrich-linear-issue-hap") + + cfg := newTestConfig(t) + log := newTestLogger() + store := newMockStore() + mgr, err := NewManager(cfg, store, log) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + + result, err := mgr.fetchBranchToLocal( + context.Background(), repoPath, "feature/enrich-linear-issue-hap", 974, + ) + if err != nil { + t.Fatalf("fetchBranchToLocal(pr=974) unexpected error: %v", err) + } + if result.Warning != "" { + t.Fatalf("expected no warning, got %q", result.Warning) + } + + gotSHA := strings.TrimSpace(runGit(t, repoPath, "rev-parse", "feature/enrich-linear-issue-hap")) + if gotSHA != prHeadSHA { + t.Fatalf("local branch SHA = %q, want %q (PR head)", gotSHA, prHeadSHA) + } +} + +// TestFetchBranchToLocal_PRNumberZeroUsesBranchFetch confirms the legacy path +// is preserved when PRNumber == 0: the manager fetches origin/, which +// is the correct behavior for non-PR tasks and same-repo branches that the +// caller knows live on origin under the same name. +func TestFetchBranchToLocal_PRNumberZeroUsesBranchFetch(t *testing.T) { + repoPath := initGitRepoWithRemote(t) + + cfg := newTestConfig(t) + log := newTestLogger() + store := newMockStore() + mgr, err := NewManager(cfg, store, log) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + + result, err := mgr.fetchBranchToLocal( + context.Background(), repoPath, "feature/pr-branch", 0, + ) + if err != nil { + t.Fatalf("fetchBranchToLocal(pr=0) unexpected error: %v", err) + } + if result.Warning != "" { + t.Fatalf("expected no warning, got %q", result.Warning) + } +} + +// TestCreateWorktree_PRNumberCreatesWorktreeFromForkRef is the end-to-end test +// for fork PRs: only the pull//head ref exists on origin (no branch named +// after the head ref). Worktree creation must still succeed by using the PR +// refspec internally, and the worktree must point at the PR head commit. +func TestCreateWorktree_PRNumberCreatesWorktreeFromForkRef(t *testing.T) { + repoPath, prHeadSHA := initGitRepoWithPullRef(t, 974, "feature/enrich-linear-issue-hap") + + cfg := newTestConfig(t) + log := newTestLogger() + store := newMockStore() + mgr, err := NewManager(cfg, store, log) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + + wt, err := mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-fork-pr", + SessionID: "session-fork-pr", + TaskTitle: "Fork PR review", + RepositoryID: "repo-1", + RepositoryPath: repoPath, + BaseBranch: "main", + CheckoutBranch: "feature/enrich-linear-issue-hap", + PRNumber: 974, + TaskDirName: "task-fork-pr", + RepoName: "repo-1", + }) + if err != nil { + t.Fatalf("Create() with PRNumber should succeed for fork PR, got: %v", err) + } + if wt.Branch != "feature/enrich-linear-issue-hap" { + t.Fatalf("expected branch %q, got %q", "feature/enrich-linear-issue-hap", wt.Branch) + } + + worktreeSHA := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD")) + if worktreeSHA != prHeadSHA { + t.Fatalf("worktree HEAD SHA = %q, want %q (PR head)", worktreeSHA, prHeadSHA) + } +} + +// TestCreateWorktree_PRNumberSecondWorktreeForSameForkPR exercises the +// retry-path regression flagged in PR #990 review: when a fork PR's head +// branch is already checked out in another worktree, the refspec fetch is +// refused and the manager retries with a bare `pull//head` fetch. That +// retry updates FETCH_HEAD but does NOT create `origin/` because +// the branch doesn't exist on origin — so returning `StartPoint: +// "origin/"` would make the downstream `git worktree add` fail +// with an invalid ref. Empty StartPoint must be returned so the caller +// falls back to the local branch (already at the PR head from task 1). +func TestCreateWorktree_PRNumberSecondWorktreeForSameForkPR(t *testing.T) { + repoPath, prHeadSHA := initGitRepoWithPullRef(t, 974, "feature/fork-pr") + + cfg := newTestConfig(t) + log := newTestLogger() + store := newMockStore() + mgr, err := NewManager(cfg, store, log) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + + wt1, err := mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-1", SessionID: "session-1", TaskTitle: "Fork PR review 1", + RepositoryID: "repo-1", RepositoryPath: repoPath, + BaseBranch: "main", CheckoutBranch: "feature/fork-pr", PRNumber: 974, + TaskDirName: "task-1", RepoName: "repo-1", + }) + if err != nil { + t.Fatalf("Create() first fork-PR worktree failed: %v", err) + } + if wt1.Branch != "feature/fork-pr" { + t.Fatalf("first worktree: expected direct branch, got %q", wt1.Branch) + } + + wt2, err := mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-2", SessionID: "session-2", TaskTitle: "Fork PR review 2", + RepositoryID: "repo-1", RepositoryPath: repoPath, + BaseBranch: "main", CheckoutBranch: "feature/fork-pr", PRNumber: 974, + TaskDirName: "task-2", RepoName: "repo-1", + }) + if err != nil { + t.Fatalf("Create() second fork-PR worktree failed (regression in retry path): %v", err) + } + if wt2.Branch == "feature/fork-pr" { + t.Fatal("second worktree must NOT reuse the branch already checked out") + } + if !strings.HasPrefix(wt2.Branch, "feature/fork-pr-") { + t.Fatalf("expected suffixed branch like %q, got %q", "feature/fork-pr-xxx", wt2.Branch) + } + + sha1 := strings.TrimSpace(runGit(t, wt1.Path, "rev-parse", "HEAD")) + sha2 := strings.TrimSpace(runGit(t, wt2.Path, "rev-parse", "HEAD")) + if sha1 != prHeadSHA || sha2 != prHeadSHA { + t.Fatalf("worktree SHAs must equal PR head: sha1=%q, sha2=%q, want=%q", + sha1, sha2, prHeadSHA) + } +} + +// initGitRepoWithPullRef simulates a fork PR scenario: a bare origin repo +// containing main + an arbitrary commit registered under refs/pull//head +// (but NOT under any branch). Returns the clone path and the PR head SHA. +func initGitRepoWithPullRef(t *testing.T, prNumber int, headBranchName string) (string, string) { + t.Helper() + + bareDir := filepath.Join(t.TempDir(), "origin.git") + runGit(t, t.TempDir(), "init", "--bare", "-b", "main", bareDir) + + cloneDir := filepath.Join(t.TempDir(), "clone") + cmd := exec.Command("git", "clone", bareDir, cloneDir) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git clone failed: %v\n%s", err, out) + } + runGit(t, cloneDir, "config", "user.email", "test@example.com") + runGit(t, cloneDir, "config", "user.name", "Test User") + runGit(t, cloneDir, "config", "commit.gpgsign", "false") + + filePath := filepath.Join(cloneDir, "README.md") + if err := os.WriteFile(filePath, []byte("initial\n"), 0644); err != nil { + t.Fatalf("failed to write README.md: %v", err) + } + runGit(t, cloneDir, "add", "README.md") + runGit(t, cloneDir, "commit", "-m", "initial commit") + runGit(t, cloneDir, "push", "origin", "main") + + // Build the PR head commit on a temporary branch, then publish it ONLY + // under refs/pull//head on the bare origin. The branch name the + // contributor used does not exist on origin — matching the fork PR + // scenario where the head branch lives only on the contributor's fork. + runGit(t, cloneDir, "checkout", "-b", "tmp-pr-head") + if err := os.WriteFile(filePath, []byte("pr change\n"), 0644); err != nil { + t.Fatalf("failed to update README.md: %v", err) + } + runGit(t, cloneDir, "add", "README.md") + runGit(t, cloneDir, "commit", "-m", "pr head commit") + prHeadSHA := strings.TrimSpace(runGit(t, cloneDir, "rev-parse", "HEAD")) + + // Push the commit object straight into origin under refs/pull//head. + // This mirrors how GitHub mirrors fork-PR head commits onto the base + // repository: the contributor's branch name is never registered, but + // the head ref is. Using `update-ref` on the bare repo directly would + // fail because the object only lives in the clone's loose objects. + runGit(t, cloneDir, "push", "origin", "HEAD:"+pullHeadRef(prNumber)) + + // Clean up the helper branch and go back to main; the PR head must NOT + // be reachable by a branch name on origin. + runGit(t, cloneDir, "checkout", "main") + runGit(t, cloneDir, "branch", "-D", "tmp-pr-head") + + // Sanity check: no remote branch matching the head ref exists. If this + // assertion ever fails, the test would silently pass via the legacy + // branch-name fetch and stop exercising the PR refspec path. + out, _ := exec.Command("git", "-C", cloneDir, "ls-remote", "--heads", "origin", headBranchName).CombinedOutput() + if strings.TrimSpace(string(out)) != "" { + t.Fatalf("test setup leaked branch %q on origin: %s", headBranchName, out) + } + + return cloneDir, prHeadSHA +} + +func pullHeadRef(prNumber int) string { + return fmt.Sprintf("refs/pull/%d/head", prNumber) +} diff --git a/apps/backend/internal/worktree/manager_test.go b/apps/backend/internal/worktree/manager_test.go index 43131c1796..a93b8c6304 100644 --- a/apps/backend/internal/worktree/manager_test.go +++ b/apps/backend/internal/worktree/manager_test.go @@ -834,7 +834,7 @@ esac } repoPath := t.TempDir() - result, err := mgr.fetchBranchToLocal(context.Background(), repoPath, "feature/pr-branch") + result, err := mgr.fetchBranchToLocal(context.Background(), repoPath, "feature/pr-branch", 0) if err != nil { t.Fatalf("fetchBranchToLocal() unexpected error: %v", err) } @@ -873,7 +873,7 @@ esac } repoPath := t.TempDir() - result, err := mgr.fetchBranchToLocal(context.Background(), repoPath, "feature/pr-branch") + result, err := mgr.fetchBranchToLocal(context.Background(), repoPath, "feature/pr-branch", 0) if err != nil { t.Fatalf("fetchBranchToLocal() should fall back to local branch, got error: %v", err) } @@ -918,7 +918,7 @@ esac } repoPath := t.TempDir() - _, err = mgr.fetchBranchToLocal(context.Background(), repoPath, "feature/pr-branch") + _, err = mgr.fetchBranchToLocal(context.Background(), repoPath, "feature/pr-branch", 0) if err == nil { t.Fatal("fetchBranchToLocal() should fail when branch not found anywhere") } @@ -957,7 +957,7 @@ esac } repoPath := t.TempDir() - _, err = mgr.fetchBranchToLocal(context.Background(), repoPath, "feature/pr-branch") + _, err = mgr.fetchBranchToLocal(context.Background(), repoPath, "feature/pr-branch", 0) if err == nil { t.Fatal("fetchBranchToLocal() should fail when remote ref is missing and no local branch") } diff --git a/apps/backend/internal/worktree/worktree.go b/apps/backend/internal/worktree/worktree.go index c592800cb5..c735f2ae9c 100644 --- a/apps/backend/internal/worktree/worktree.go +++ b/apps/backend/internal/worktree/worktree.go @@ -119,6 +119,15 @@ type CreateRequest struct { // fallback branch is created using the original name with a random suffix. CheckoutBranch string + // PRNumber, when > 0, identifies the GitHub PR whose head ref should be + // fetched into the local CheckoutBranch via the refs/pull//head refspec. + // Required for fork PRs, whose branches only exist on the contributor's + // fork — git fetch origin fails because origin (the base repo) + // has no such ref. GitHub mirrors every PR head under refs/pull//head + // on the base repo, so the refspec form works for same-repo and fork PRs + // uniformly without needing to add the fork as a remote. + PRNumber int + // WorktreeBranchPrefix is the prefix to use for the worktree branch name. // If empty, the default prefix is used. WorktreeBranchPrefix string diff --git a/apps/web/components/task-create-dialog-effects.test.ts b/apps/web/components/task-create-dialog-effects.test.ts index 97318949c6..1b299e87bd 100644 --- a/apps/web/components/task-create-dialog-effects.test.ts +++ b/apps/web/components/task-create-dialog-effects.test.ts @@ -1,6 +1,9 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { renderHook } from "@testing-library/react"; -import { useWorkflowAgentProfileEffect } from "./task-create-dialog-effects"; +import { + useBranchAutoSelectEffect, + useWorkflowAgentProfileEffect, +} from "./task-create-dialog-effects"; import type { DialogFormState } from "@/components/task-create-dialog-types"; import type { AgentProfileOption } from "@/lib/state/slices"; import { STORAGE_KEYS } from "@/lib/settings/constants"; @@ -106,3 +109,81 @@ describe("useWorkflowAgentProfileEffect", () => { expect(fs.setAgentProfileId).toHaveBeenCalledWith(""); }); }); + +type BranchFake = Pick< + DialogFormState, + "githubBranch" | "githubBranches" | "useGitHubUrl" | "setGitHubBranch" | "githubPrHeadBranch" +>; +function makeBranchFs(overrides: Partial = {}): DialogFormState { + return { + githubBranch: "", + githubBranches: [], + useGitHubUrl: true, + setGitHubBranch: vi.fn(), + githubPrHeadBranch: null, + ...overrides, + } as unknown as DialogFormState; +} + +describe("useBranchAutoSelectEffect", () => { + it("selects the PR head branch when it is present in the base repo's branch list", () => { + const fs = makeBranchFs({ + githubBranches: [ + { name: "main", type: "remote" }, + { name: "feature/x", type: "remote" }, + ], + githubPrHeadBranch: "feature/x", + }); + renderHook(() => useBranchAutoSelectEffect(fs)); + expect(fs.setGitHubBranch).toHaveBeenCalledWith("feature/x"); + }); + + it("still surfaces the PR head branch for fork PRs whose head is NOT in the base repo's branch list", () => { + // Regression guard for the fork-PR display bug: previously the effect + // fell through to "main" when the PR head was missing from the base + // repo's branches, visually contradicting the URL the user just pasted. + const fs = makeBranchFs({ + githubBranches: [ + { name: "main", type: "remote" }, + { name: "develop", type: "remote" }, + ], + githubPrHeadBranch: "jira-hosted-path-auth", + }); + renderHook(() => useBranchAutoSelectEffect(fs)); + expect(fs.setGitHubBranch).toHaveBeenCalledWith("jira-hosted-path-auth"); + }); + + it("falls back to main when there is no PR head branch", () => { + const fs = makeBranchFs({ + githubBranches: [ + { name: "feature/y", type: "remote" }, + { name: "main", type: "remote" }, + ], + }); + renderHook(() => useBranchAutoSelectEffect(fs)); + expect(fs.setGitHubBranch).toHaveBeenCalledWith("main"); + }); + + it("does nothing when useGitHubUrl is false", () => { + const fs = makeBranchFs({ + useGitHubUrl: false, + githubBranches: [{ name: "main", type: "remote" }], + githubPrHeadBranch: "feature/x", + }); + renderHook(() => useBranchAutoSelectEffect(fs)); + expect(fs.setGitHubBranch).not.toHaveBeenCalled(); + }); + + it("does nothing once a branch has already been selected (manual override)", () => { + const fs = makeBranchFs({ + githubBranch: "develop", + githubBranches: [ + { name: "main", type: "remote" }, + { name: "develop", type: "remote" }, + ], + githubPrHeadBranch: "feature/x", + }); + renderHook(() => useBranchAutoSelectEffect(fs)); + expect(fs.setGitHubBranch).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/components/task-create-dialog-effects.ts b/apps/web/components/task-create-dialog-effects.ts index 8274a57aa9..b6d16133ef 100644 --- a/apps/web/components/task-create-dialog-effects.ts +++ b/apps/web/components/task-create-dialog-effects.ts @@ -367,14 +367,18 @@ export function useDefaultSelectionsEffect( export function useBranchAutoSelectEffect(fs: DialogFormState) { const { githubBranch, githubBranches, useGitHubUrl, setGitHubBranch, githubPrHeadBranch } = fs; useEffect(() => { - if (!useGitHubUrl || githubBranches.length === 0 || githubBranch) return; + if (!useGitHubUrl || githubBranch) return; + // PR head wins regardless of whether it appears in the base repo's branch + // list. Fork PRs (head lives only on the contributor's fork) won't match + // anything in githubBranches, but we still want the pill to surface the + // PR's branch name — the worktree will materialize it from the PR refspec + // on the backend, and falling through to "main" would visually contradict + // the URL the user just pasted. if (githubPrHeadBranch) { - const prBranch = githubBranches.find((b) => b.name === githubPrHeadBranch); - if (prBranch) { - setGitHubBranch(prBranch.name); - return; - } + setGitHubBranch(githubPrHeadBranch); + return; } + if (githubBranches.length === 0) return; // GitHub URL branches are referenced by name only (no remote prefix); // selectPreferredBranch expects origin-prefixed remotes, so pick directly. const preferred = @@ -412,6 +416,7 @@ export function useGitHubUrlBranchesEffect(fs: DialogFormState, open: boolean) { setGitHubBranchesLoading, setGitHubUrlError, setGitHubPrHeadBranch, + setGitHubPrBaseBranch, } = fs; useEffect(() => { if (!open || !useGitHubUrl) { @@ -429,6 +434,7 @@ export function useGitHubUrlBranchesEffect(fs: DialogFormState, open: boolean) { if (!parsed) { setGitHubBranches([]); setGitHubPrHeadBranch(null); + setGitHubPrBaseBranch(null); setGitHubBranchesLoading(false); setGitHubUrlError("Invalid GitHub URL — expected github.com/owner/repo or .../pull/123"); return; @@ -437,6 +443,7 @@ export function useGitHubUrlBranchesEffect(fs: DialogFormState, open: boolean) { setGitHubUrlError(null); setGitHubBranchesLoading(true); setGitHubPrHeadBranch(null); + setGitHubPrBaseBranch(null); const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), 15_000); @@ -456,6 +463,7 @@ export function useGitHubUrlBranchesEffect(fs: DialogFormState, open: boolean) { setGitHubUrlError(null); if (prInfo) { setGitHubPrHeadBranch(prInfo.head_branch); + setGitHubPrBaseBranch(prInfo.base_branch); } }) .catch((err) => { @@ -488,6 +496,7 @@ export function useGitHubUrlBranchesEffect(fs: DialogFormState, open: boolean) { setGitHubBranchesLoading, setGitHubUrlError, setGitHubPrHeadBranch, + setGitHubPrBaseBranch, ]); } diff --git a/apps/web/components/task-create-dialog-github-url.tsx b/apps/web/components/task-create-dialog-github-url.tsx index 2f0a32f88b..cebbb24719 100644 --- a/apps/web/components/task-create-dialog-github-url.tsx +++ b/apps/web/components/task-create-dialog-github-url.tsx @@ -101,13 +101,13 @@ function GitHubUrlPill({ type="text" value={githubUrl} onChange={(e) => onGitHubUrlChange?.(e.target.value)} - placeholder="github.com/owner/repo" + placeholder="github.com/owner/repo or .../pull/123" data-testid="github-url-input" - aria-label="GitHub repository URL" + aria-label="GitHub repository or PR URL" aria-invalid={!!githubUrlError} aria-describedby={githubUrlError ? "github-url-error" : undefined} className={cn( - "h-7 rounded-md px-2.5 text-xs bg-muted/30 border border-border/60", + "h-7 w-[320px] rounded-md px-2.5 text-xs bg-muted/30 border border-border/60", "outline-none focus:bg-muted focus:border-border placeholder:text-muted-foreground", githubUrlError && "border-destructive text-destructive", )} diff --git a/apps/web/components/task-create-dialog-helpers.multi-repo.test.ts b/apps/web/components/task-create-dialog-helpers.multi-repo.test.ts index aee02224a6..b43e88daf3 100644 --- a/apps/web/components/task-create-dialog-helpers.multi-repo.test.ts +++ b/apps/web/components/task-create-dialog-helpers.multi-repo.test.ts @@ -224,4 +224,73 @@ describe("buildRepositoriesPayload — single-row and URL mode", () => { { repository_id: "repo-only", base_branch: "main", checkout_branch: undefined }, ]); }); + + // Fork PR: the displayed branch equals the PR head (auto-selected for visual + // consistency with the pasted URL), but that branch doesn't live on origin. + // The payload must anchor base_branch to the PR's actual target (from the + // GitHub API) so the backend has a ref it can resolve. + it("fork PR auto-selection: base_branch comes from PR's target, not displayed branch", () => { + const payload = buildRepositoriesPayload({ + useGitHubUrl: true, + githubUrl: "https://github.com/kdlbs/kandev/pull/977", + githubBranch: "jira-hosted-path-auth", // displayed = PR head + githubPrHeadBranch: "jira-hosted-path-auth", + githubPrBaseBranch: "main", + repositories: [], + discoveredRepositories: [], + }); + expect(payload).toEqual([ + { + repository_id: "", + base_branch: "main", + checkout_branch: "jira-hosted-path-auth", + github_url: "https://github.com/kdlbs/kandev/pull/977", + }, + ]); + }); + + // User picked a non-PR-head branch from the dropdown after pasting a PR URL. + // We respect their override: send their pick as base, keep PR head as checkout. + it("user-overridden base branch beats PR's target when displayed branch differs from PR head", () => { + const payload = buildRepositoriesPayload({ + useGitHubUrl: true, + githubUrl: "https://github.com/owner/repo/pull/42", + githubBranch: "develop", // user picked something else + githubPrHeadBranch: "feature/x", + githubPrBaseBranch: "main", + repositories: [], + discoveredRepositories: [], + }); + expect(payload).toEqual([ + { + repository_id: "", + base_branch: "develop", + checkout_branch: "feature/x", + github_url: "https://github.com/owner/repo/pull/42", + }, + ]); + }); + + // Same-repo PR: PR head exists on origin, so base_branch = PR head is fine. + // PR's base from API is still preferred when available, so the auto-selected + // case still anchors to the PR's actual target rather than the head. + it("same-repo PR auto-selection: still prefers PR target over PR head", () => { + const payload = buildRepositoriesPayload({ + useGitHubUrl: true, + githubUrl: "https://github.com/owner/repo/pull/10", + githubBranch: "feature/x", + githubPrHeadBranch: "feature/x", + githubPrBaseBranch: "main", + repositories: [], + discoveredRepositories: [], + }); + expect(payload).toEqual([ + { + repository_id: "", + base_branch: "main", + checkout_branch: "feature/x", + github_url: "https://github.com/owner/repo/pull/10", + }, + ]); + }); }); diff --git a/apps/web/components/task-create-dialog-helpers.ts b/apps/web/components/task-create-dialog-helpers.ts index 6f5a67e798..39b2a0cd5a 100644 --- a/apps/web/components/task-create-dialog-helpers.ts +++ b/apps/web/components/task-create-dialog-helpers.ts @@ -173,6 +173,13 @@ export function buildRepositoriesPayload(opts: { githubUrl: string; githubBranch: string; githubPrHeadBranch: string | null; + /** + * PR's target branch from the GitHub API. When set and the displayed branch + * still matches the auto-selected PR head, this is sent as `base_branch` + * (so origin can resolve it for fork PRs). Optional to keep existing test + * fixtures and non-PR call sites compiling without a sentinel. + */ + githubPrBaseBranch?: string | null; repositories: TaskRepoRow[]; /** Used to look up `default_branch` for `localPath` rows. */ discoveredRepositories: LocalRepository[]; @@ -195,10 +202,21 @@ export function buildRepositoriesPayload(opts: { freshBranch?: { confirmDiscard: boolean; consentedDirtyFiles: string[] }; }): NonNullable { if (opts.useGitHubUrl && opts.githubUrl) { + // For PR URLs we display the PR head branch in the pill (so the user sees + // the branch they pasted). The displayed branch is NOT a valid base on + // origin for fork PRs — it only exists on the contributor's fork — so we + // anchor `base_branch` to the PR's actual target branch from the GitHub + // API and let `checkout_branch` carry the head ref. The backend materializes + // the head via the refs/pull//head refspec. + const isPrAutoSelection = + !!opts.githubPrHeadBranch && opts.githubBranch === opts.githubPrHeadBranch; + const baseBranch = isPrAutoSelection + ? opts.githubPrBaseBranch || undefined + : opts.githubBranch || undefined; return [ { repository_id: "", - base_branch: opts.githubBranch || undefined, + base_branch: baseBranch, checkout_branch: opts.githubPrHeadBranch || undefined, github_url: opts.githubUrl.trim(), }, diff --git a/apps/web/components/task-create-dialog-setup.tsx b/apps/web/components/task-create-dialog-setup.tsx index 4b1d1e1315..7d0f390ca8 100644 --- a/apps/web/components/task-create-dialog-setup.tsx +++ b/apps/web/components/task-create-dialog-setup.tsx @@ -177,6 +177,7 @@ function useSubmitHandlersWiring({ useGitHubUrl: fs.useGitHubUrl, githubUrl: fs.githubUrl, githubPrHeadBranch: fs.githubPrHeadBranch, + githubPrBaseBranch: fs.githubPrBaseBranch, githubBranch: fs.githubBranch, agentProfileId: computed.effectiveAgentProfileId, executorId: fs.executorId, diff --git a/apps/web/components/task-create-dialog-state.ts b/apps/web/components/task-create-dialog-state.ts index 7e4fead44f..b8e8bad5c4 100644 --- a/apps/web/components/task-create-dialog-state.ts +++ b/apps/web/components/task-create-dialog-state.ts @@ -98,6 +98,7 @@ type FormResetters = { setGitHubBranches: (v: Branch[]) => void; setGitHubUrlError: (v: string | null) => void; setGitHubPrHeadBranch: (v: string | null) => void; + setGitHubPrBaseBranch: (v: string | null) => void; setFreshBranchEnabled: (v: boolean) => void; setCurrentLocalBranch: (v: string) => void; }; @@ -216,6 +217,7 @@ function resetDiscoveryState(resetters: FormResetters, iv?: TaskCreateDialogInit resetters.setGitHubBranches([]); resetters.setGitHubUrlError(null); resetters.setGitHubPrHeadBranch(iv?.checkoutBranch ?? null); + resetters.setGitHubPrBaseBranch(null); resetters.setFreshBranchEnabled(false); resetters.setCurrentLocalBranch(""); // Source-mode toggle resets — without these, opening the dialog in "None" @@ -293,6 +295,7 @@ function useGitHubUrlState() { const [githubBranchesLoading, setGitHubBranchesLoading] = useState(false); const [githubUrlError, setGitHubUrlError] = useState(null); const [githubPrHeadBranch, setGitHubPrHeadBranch] = useState(null); + const [githubPrBaseBranch, setGitHubPrBaseBranch] = useState(null); return { useGitHubUrl, setUseGitHubUrl, @@ -306,6 +309,8 @@ function useGitHubUrlState() { setGitHubUrlError, githubPrHeadBranch, setGitHubPrHeadBranch, + githubPrBaseBranch, + setGitHubPrBaseBranch, }; } @@ -446,6 +451,7 @@ export function useDialogFormState( setGitHubBranches: ghUrl.setGitHubBranches, setGitHubUrlError: ghUrl.setGitHubUrlError, setGitHubPrHeadBranch: ghUrl.setGitHubPrHeadBranch, + setGitHubPrBaseBranch: ghUrl.setGitHubPrBaseBranch, setFreshBranchEnabled: freshBranch.setFreshBranchEnabled, setCurrentLocalBranch: freshBranch.setCurrentLocalBranch, setNoRepository: form.setNoRepository, diff --git a/apps/web/components/task-create-dialog-submit.tsx b/apps/web/components/task-create-dialog-submit.tsx index 25988395e1..6babc8b7f5 100644 --- a/apps/web/components/task-create-dialog-submit.tsx +++ b/apps/web/components/task-create-dialog-submit.tsx @@ -37,6 +37,7 @@ export function useTaskSubmitHandlers({ useGitHubUrl, githubUrl, githubPrHeadBranch, + githubPrBaseBranch, githubBranch, agentProfileId, executorId, @@ -131,6 +132,7 @@ export function useTaskSubmitHandlers({ githubUrl, githubBranch, githubPrHeadBranch, + githubPrBaseBranch, repositories, discoveredRepositories, workspaceRepositories, @@ -146,6 +148,7 @@ export function useTaskSubmitHandlers({ githubUrl, githubBranch, githubPrHeadBranch, + githubPrBaseBranch, repositories, discoveredRepositories, workspaceRepositories, diff --git a/apps/web/components/task-create-dialog-types.ts b/apps/web/components/task-create-dialog-types.ts index ffc0c68d01..6de7f643a4 100644 --- a/apps/web/components/task-create-dialog-types.ts +++ b/apps/web/components/task-create-dialog-types.ts @@ -205,6 +205,14 @@ export type DialogFormState = { setGitHubUrlError: (v: string | null) => void; githubPrHeadBranch: string | null; setGitHubPrHeadBranch: (v: string | null) => void; + /** + * PR's target/base branch from the GitHub API (e.g. "main"). Captured so the + * payload can send a base_branch that resolves on origin, even when the PR + * head only lives on a fork. Without this, fork PRs would propagate the + * head-branch name into base_branch and the worktree would fail to anchor. + */ + githubPrBaseBranch: string | null; + setGitHubPrBaseBranch: (v: string | null) => void; /** When non-empty, the selected workflow overrides the agent profile */ workflowAgentProfileId: string; setWorkflowAgentProfileId: (v: string) => void; @@ -245,6 +253,7 @@ export type SubmitHandlersDeps = { useGitHubUrl: boolean; githubUrl: string; githubPrHeadBranch: string | null; + githubPrBaseBranch: string | null; /** Branch for the GitHub URL flow. Per-row branches live on `repositories[i].branch`. */ githubBranch: string; agentProfileId: string; diff --git a/apps/web/components/task-create-dialog.tsx b/apps/web/components/task-create-dialog.tsx index ce8c2de32d..6bb9a9e6e5 100644 --- a/apps/web/components/task-create-dialog.tsx +++ b/apps/web/components/task-create-dialog.tsx @@ -330,6 +330,7 @@ function useSubmitHandlersWiring({ useGitHubUrl: fs.useGitHubUrl, githubUrl: fs.githubUrl, githubPrHeadBranch: fs.githubPrHeadBranch, + githubPrBaseBranch: fs.githubPrBaseBranch, githubBranch: fs.githubBranch, agentProfileId: computed.effectiveAgentProfileId, executorId: fs.executorId, diff --git a/apps/web/components/task/new-subtask-form-state.ts b/apps/web/components/task/new-subtask-form-state.ts index 953522977e..b49dad13de 100644 --- a/apps/web/components/task/new-subtask-form-state.ts +++ b/apps/web/components/task/new-subtask-form-state.ts @@ -52,6 +52,7 @@ export function useSubtaskFormState(): DialogFormState { const [githubBranchesLoading, setGitHubBranchesLoading] = useState(false); const [githubUrlError, setGitHubUrlError] = useState(null); const [githubPrHeadBranch, setGitHubPrHeadBranch] = useState(null); + const [githubPrBaseBranch, setGitHubPrBaseBranch] = useState(null); // Discovered (on-disk) repos — populated by useDiscoverReposEffect when the // dialog opens, same as the create-task flow. This lets users target // not-yet-imported on-machine git folders for the subtask. @@ -106,6 +107,8 @@ export function useSubtaskFormState(): DialogFormState { setGitHubUrlError, githubPrHeadBranch, setGitHubPrHeadBranch, + githubPrBaseBranch, + setGitHubPrBaseBranch, workflowAgentProfileId: "", setWorkflowAgentProfileId: NOOP, clearDraft: NOOP, @@ -126,6 +129,7 @@ export function useSubtaskFormState(): DialogFormState { githubBranchesLoading, githubUrlError, githubPrHeadBranch, + githubPrBaseBranch, discoveredRepositories, discoverReposLoading, discoverReposLoaded, diff --git a/apps/web/components/task/use-subtask-submit.ts b/apps/web/components/task/use-subtask-submit.ts index 50ea0e3f21..3d23198da9 100644 --- a/apps/web/components/task/use-subtask-submit.ts +++ b/apps/web/components/task/use-subtask-submit.ts @@ -70,6 +70,7 @@ export function useSubtaskSubmit(opts: UseSubtaskSubmitOpts) { githubUrl: fs.githubUrl, githubBranch: fs.githubBranch, githubPrHeadBranch: fs.githubPrHeadBranch, + githubPrBaseBranch: fs.githubPrBaseBranch, repositories: fs.repositories, discoveredRepositories: fs.discoveredRepositories, workspaceRepositories: availableRepositories,