fix(worktree): support fork PR worktree creation via pull refspec (#990)
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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{
|
||||
|
||||
@@ -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/<N>/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/<N>/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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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/<N>/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/<N>/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,
|
||||
|
||||
@@ -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/<N>/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
|
||||
|
||||
@@ -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/<N>/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/<N>/head fetch for fork PRs.
|
||||
WorktreeID string
|
||||
WorktreeBranchPrefix string
|
||||
PullBeforeWorktree bool
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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/<N> 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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -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"`
|
||||
|
||||
@@ -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/<N> 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/<N>/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
|
||||
|
||||
@@ -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/<N>/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/<branch> ref — the bare
|
||||
// pull/<N>/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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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/<N>/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/<N>/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/<branch>, 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/<N>/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/<N>/head` fetch. That
|
||||
// retry updates FETCH_HEAD but does NOT create `origin/<branch>` because
|
||||
// the branch doesn't exist on origin — so returning `StartPoint:
|
||||
// "origin/<branch>"` 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/<N>/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/<N>/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/<N>/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)
|
||||
}
|
||||
@@ -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")
|
||||
}
|
||||
|
||||
@@ -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/<N>/head refspec.
|
||||
// Required for fork PRs, whose branches only exist on the contributor's
|
||||
// fork — git fetch origin <branch> fails because origin (the base repo)
|
||||
// has no such ref. GitHub mirrors every PR head under refs/pull/<N>/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
|
||||
|
||||
@@ -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<BranchFake> = {}): 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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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,
|
||||
]);
|
||||
}
|
||||
|
||||
|
||||
@@ -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",
|
||||
)}
|
||||
|
||||
@@ -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",
|
||||
},
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<CreateTaskParams["repositories"]> {
|
||||
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/<N>/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(),
|
||||
},
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<string | null>(null);
|
||||
const [githubPrHeadBranch, setGitHubPrHeadBranch] = useState<string | null>(null);
|
||||
const [githubPrBaseBranch, setGitHubPrBaseBranch] = useState<string | null>(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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -52,6 +52,7 @@ export function useSubtaskFormState(): DialogFormState {
|
||||
const [githubBranchesLoading, setGitHubBranchesLoading] = useState(false);
|
||||
const [githubUrlError, setGitHubUrlError] = useState<string | null>(null);
|
||||
const [githubPrHeadBranch, setGitHubPrHeadBranch] = useState<string | null>(null);
|
||||
const [githubPrBaseBranch, setGitHubPrBaseBranch] = useState<string | null>(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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user