fix(project): resolve default branch from origin/HEAD, not checked-out branch (#289)
* fix(project): resolve default branch from origin/HEAD, not checked-out branch Detecting the project default via `symbolic-ref --short HEAD` captured whatever branch the repo happened to be on at add time. Adding a project while on a feature branch (e.g. fix/pr-attachment) persisted that branch as the default, so every session worktree based off it instead of main. Prefer the remote default (origin/HEAD), falling back to the checked-out branch only when no remote default is set. This still records a non-main default like master correctly, while ignoring the active feature branch. * test: cover branch-not-fetched API error
This commit is contained in:
parent
43ee6c9b02
commit
7d0ca02440
|
|
@ -23,6 +23,7 @@ type fakeSessionService struct {
|
|||
cleanupProjects []domain.ProjectID
|
||||
cleanupResult []domain.SessionID
|
||||
cleanupSkipped []sessionsvc.CleanupSkipped
|
||||
spawnErr error
|
||||
claimErr error
|
||||
listPRErr error
|
||||
}
|
||||
|
|
@ -51,6 +52,9 @@ func (f *fakeSessionService) List(_ context.Context, filter sessionsvc.ListFilte
|
|||
}
|
||||
|
||||
func (f *fakeSessionService) Spawn(_ context.Context, cfg ports.SpawnConfig) (domain.Session, error) {
|
||||
if f.spawnErr != nil {
|
||||
return domain.Session{}, f.spawnErr
|
||||
}
|
||||
now := time.Now().UTC()
|
||||
s := domain.Session{SessionRecord: domain.SessionRecord{ID: domain.SessionID(string(cfg.ProjectID) + "-2"), ProjectID: cfg.ProjectID, IssueID: cfg.IssueID, Kind: cfg.Kind, Harness: cfg.Harness, Activity: domain.Activity{State: domain.ActivityIdle, LastActivityAt: now}, CreatedAt: now, UpdatedAt: now}, Status: domain.StatusIdle}
|
||||
f.sessions[s.ID] = s
|
||||
|
|
@ -243,6 +247,15 @@ func TestSessionsAPI_ListSpawnGetAndActions(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestSessionsAPI_SpawnBranchNotFetchedReturnsTypedError(t *testing.T) {
|
||||
svc := newFakeSessionService()
|
||||
svc.spawnErr = apierr.Invalid("BRANCH_NOT_FETCHED", `workspace: branch is not fetched: "feature/missing"`, nil)
|
||||
srv := newSessionTestServer(t, svc)
|
||||
|
||||
body, status, _ := doRequest(t, srv, "POST", "/api/v1/sessions", `{"projectId":"ao","kind":"worker","branch":"feature/missing","prompt":"fix"}`)
|
||||
assertErrorCode(t, body, status, http.StatusBadRequest, "BRANCH_NOT_FETCHED")
|
||||
}
|
||||
|
||||
func TestSessionsAPI_RenameNotFound(t *testing.T) {
|
||||
srv := newSessionTestServer(t, newFakeSessionService())
|
||||
|
||||
|
|
|
|||
|
|
@ -301,12 +301,25 @@ func resolveGitOriginURL(path string) string {
|
|||
return strings.TrimSpace(string(out))
|
||||
}
|
||||
|
||||
// resolveDefaultBranch returns the repo's currently checked-out branch via
|
||||
// `git -C path symbolic-ref --short HEAD`. A detached HEAD, missing repo, or any
|
||||
// other git error returns an empty string — `project add` must not fail just
|
||||
// because the branch can't be resolved (the caller falls back to
|
||||
// DefaultBranchName).
|
||||
// resolveDefaultBranch returns the repo's default branch, preferring the
|
||||
// remote's default (`origin/HEAD`) over the currently checked-out branch. This
|
||||
// matters because the user may have the repo on a feature branch when adding the
|
||||
// project: keying off HEAD would persist that feature branch as the project
|
||||
// default and base every session worktree on it. `origin/HEAD` reflects the
|
||||
// real default (e.g. `master`, `develop`) regardless of the active branch.
|
||||
//
|
||||
// Falls back to the checked-out branch when origin/HEAD is unset (no remote, or
|
||||
// it was never fetched). A detached HEAD, missing repo, or any other git error
|
||||
// returns an empty string — `project add` must not fail just because the branch
|
||||
// can't be resolved (the caller falls back to DefaultBranchName).
|
||||
func resolveDefaultBranch(path string) string {
|
||||
if out, err := exec.Command(
|
||||
"git", "-C", path, "symbolic-ref", "--short", "refs/remotes/origin/HEAD",
|
||||
).Output(); err == nil {
|
||||
if ref := strings.TrimSpace(string(out)); ref != "" {
|
||||
return strings.TrimPrefix(ref, "origin/")
|
||||
}
|
||||
}
|
||||
out, err := exec.Command("git", "-C", path, "symbolic-ref", "--short", "HEAD").Output()
|
||||
if err != nil {
|
||||
return ""
|
||||
|
|
|
|||
|
|
@ -52,6 +52,32 @@ func gitRepoOnBranch(t *testing.T, branch string) string {
|
|||
return dir
|
||||
}
|
||||
|
||||
// gitRepoWithOriginHead creates a repo whose remote default (origin/HEAD) points
|
||||
// at defaultBranch while the working tree is checked out on featureBranch. This
|
||||
// mirrors a user adding a project while sitting on a feature branch: detection
|
||||
// must record the remote default, not the active branch.
|
||||
func gitRepoWithOriginHead(t *testing.T, defaultBranch, featureBranch string) string {
|
||||
t.Helper()
|
||||
dir := t.TempDir()
|
||||
run := func(args ...string) {
|
||||
if out, err := exec.Command("git", append([]string{"-C", dir}, args...)...).CombinedOutput(); err != nil {
|
||||
t.Fatalf("git %v: %v (%s)", args, err, out)
|
||||
}
|
||||
}
|
||||
if out, err := exec.Command("git", "init", "-b", defaultBranch, dir).CombinedOutput(); err != nil {
|
||||
t.Fatalf("git unavailable: %v (%s)", err, out)
|
||||
}
|
||||
run("config", "user.email", "test@example.com")
|
||||
run("config", "user.name", "test")
|
||||
run("commit", "--allow-empty", "-m", "init")
|
||||
// Fabricate a remote-tracking default without a real remote: point
|
||||
// refs/remotes/origin/<defaultBranch> at HEAD, then set origin/HEAD to it.
|
||||
run("update-ref", "refs/remotes/origin/"+defaultBranch, "HEAD")
|
||||
run("symbolic-ref", "refs/remotes/origin/HEAD", "refs/remotes/origin/"+defaultBranch)
|
||||
run("checkout", "-b", featureBranch)
|
||||
return dir
|
||||
}
|
||||
|
||||
func ptr(s string) *string { return &s }
|
||||
|
||||
// wantCode asserts err is an *apierr.Error carrying the given machine code.
|
||||
|
|
@ -298,6 +324,42 @@ func TestManager_AddDetectsNonMainDefaultBranch(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// A repo checked out on a feature branch must NOT record that branch as the
|
||||
// project default — detection must prefer the remote default (origin/HEAD), so a
|
||||
// repo whose origin/HEAD is `main` stays on `main` even when HEAD is elsewhere.
|
||||
func TestManager_AddPrefersOriginHeadOverCheckedOutBranch(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
m := newManager(t)
|
||||
repo := gitRepoWithOriginHead(t, "main", "fix/pr-attachment")
|
||||
|
||||
proj, err := m.Add(ctx, project.AddInput{Path: repo, ProjectID: ptr("ao")})
|
||||
if err != nil {
|
||||
t.Fatalf("Add: %v", err)
|
||||
}
|
||||
// origin/HEAD is `main`, which equals DefaultBranchName, so config stays empty
|
||||
// and the effective default resolves to main — never the feature branch.
|
||||
if proj.DefaultBranch != domain.DefaultBranchName {
|
||||
t.Fatalf("DefaultBranch = %q, want %q (not the checked-out feature branch)",
|
||||
proj.DefaultBranch, domain.DefaultBranchName)
|
||||
}
|
||||
}
|
||||
|
||||
// When origin/HEAD points at a non-main default (e.g. master), detection records
|
||||
// that — not the feature branch the user happens to be on.
|
||||
func TestManager_AddPrefersOriginHeadNonMain(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
m := newManager(t)
|
||||
repo := gitRepoWithOriginHead(t, "master", "fix/pr-attachment")
|
||||
|
||||
proj, err := m.Add(ctx, project.AddInput{Path: repo, ProjectID: ptr("ao")})
|
||||
if err != nil {
|
||||
t.Fatalf("Add: %v", err)
|
||||
}
|
||||
if proj.DefaultBranch != "master" {
|
||||
t.Fatalf("DefaultBranch = %q, want master (origin/HEAD), not feature branch", proj.DefaultBranch)
|
||||
}
|
||||
}
|
||||
|
||||
func TestManager_SetConfig(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
m := newManager(t)
|
||||
|
|
|
|||
Loading…
Reference in New Issue