branch submit: Heal from external PR submissions (#141)

If a PR was submitted without using `gs`,
`branch submit` will detect that and heal state.

Obviates need for manually associating PRs with branches after the fact.

Resolves #62

.changes/unreleased/Changed-20240601-162128.yaml

@@ -0,0 +1,3 @@
+kind: Changed
+body: 'branch submit: Auto-detect PRs created outside git-spice, e.g. using the GitHub UI.'
+time: 2024-06-01T16:21:28.52887-07:00

branch_submit.go

@@ -112,17 +112,60 @@ func (cmd *branchSubmitCmd) Run(
 		return err
 	}
 
-	changes, err := forge.ListChanges(ctx, github.ListChangesOptions{
-		State:  "open",
-		Branch: upstreamBranch,
-		// We don't filter by base here as that may be out of date.
-	})
-	if err != nil {
-		return fmt.Errorf("list changes: %w", err)
+	// If the branch doesn't have a PR associated with it,
+	// we'll probably need to create one,
+	// but verify that there isn't already one open.
+	var existingChange *github.FindChangeItem
+	if branch.PR == 0 {
+		changes, err := forge.FindChangesByBranch(ctx, upstreamBranch)
+		if err != nil {
+			return fmt.Errorf("list changes: %w", err)
+		}
+
+		switch len(changes) {
+		case 0:
+			// No PRs found, we'll create one.
+		case 1:
+			existingChange = &changes[0]
+
+			// A PR was found, but it wasn't associated with the branch.
+			// It was probably created manually.
+			// We'll heal the state while we're at it.
+			log.Infof("%v: Found existing PR %v", cmd.Name, existingChange.ID)
+			err := store.Update(ctx, &state.UpdateRequest{
+				Upserts: []state.UpsertRequest{
+					{
+						Name: cmd.Name,
+						PR:   int(existingChange.ID),
+					},
+				},
+				Message: fmt.Sprintf("%v: associate existing PR", cmd.Name),
+			})
+			if err != nil {
+				return fmt.Errorf("update state: %w", err)
+			}
+
+		default:
+			// GitHub doesn't allow multiple PRs for the same branch
+			// with the same base branch.
+			// If we get here, it means there are multiple PRs open
+			// with different base branches.
+			return fmt.Errorf("multiple open pull requests for %s", cmd.Name)
+			// TODO: Ask the user to pick one and associate it with the branch.
+		}
+	} else {
+		// If a PR is already associated with the branch,
+		// fetch information about it to compare with the current state.
+		change, err := forge.FindChangeByID(ctx, github.ChangeID(branch.PR))
+		if err != nil {
+			return fmt.Errorf("find change: %w", err)
+		}
+		// TODO: If the PR is closed, we should treat it as non-existent.
+		existingChange = change
 	}
 
-	switch len(changes) {
-	case 0:
+	// At this point, existingChange is nil only if we need to create a new PR.
+	if existingChange == nil {
 		if cmd.DryRun {
 			log.Infof("WOULD create a pull request for %s", cmd.Name)
 			return nil
@@ -259,10 +302,9 @@ func (cmd *branchSubmitCmd) Run(
 		upsert.PR = int(result.ID)
 
 		log.Infof("Created %v: %s", result.ID, result.URL)
-
-	case 1:
+	} else {
 		// Check base and HEAD are up-to-date.
-		pull := changes[0]
+		pull := existingChange
 		var updates []string
 		if pull.HeadHash != commitHash {
 			updates = append(updates, "push branch")
@@ -317,10 +359,6 @@ func (cmd *branchSubmitCmd) Run(
 		}
 
 		log.Infof("Updated %v: %s", pull.ID, pull.URL)
-
-	default:
-		// TODO: add a --pr flag to allow picking a PR?
-		return fmt.Errorf("multiple open pull requests for %s", cmd.Name)
 	}
 
 	return nil

internal/forge/github/find.go

@@ -0,0 +1,78 @@
+package github
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/google/go-github/v61/github"
+	"go.abhg.dev/gs/internal/git"
+)
+
+// FindChangeItem is a single result from searching for changes in the
+// repository.
+type FindChangeItem struct {
+	// ID is a unique identifier for the change.
+	ID ChangeID
+
+	// URL is the web URL at which the change can be viewed.
+	URL string
+
+	// Subject is the title of the change.
+	Subject string
+
+	// HeadHash is the hash of the commit at the top of the change.
+	HeadHash git.Hash
+
+	// BaseName is the name of the base branch
+	// that this change is proposed against.
+	BaseName string
+
+	// Draft is true if the change is not yet ready to be reviewed.
+	Draft bool
+}
+
+// TODO: Reduce filtering options in favor of explicit queries,
+// e.g. "FindChangesForBranch" or "ListOpenChanges", etc.
+
+// FindChangesByBranch searches for open changes with the given branch name.
+// Returns [ErrNotFound] if no changes are found.
+func (f *Forge) FindChangesByBranch(ctx context.Context, branch string) ([]FindChangeItem, error) {
+	pulls, _, err := f.client.PullRequests.List(ctx, f.owner, f.repo, &github.PullRequestListOptions{
+		State: "open",
+		Head:  f.owner + ":" + branch,
+	})
+	if err != nil {
+		return nil, fmt.Errorf("list pull requests: %w", err)
+	}
+
+	changes := make([]FindChangeItem, len(pulls))
+	for i, pull := range pulls {
+		changes[i] = FindChangeItem{
+			ID:       ChangeID(pull.GetNumber()),
+			URL:      pull.GetHTMLURL(),
+			Subject:  pull.GetTitle(),
+			BaseName: pull.GetBase().GetRef(),
+			HeadHash: git.Hash(pull.GetHead().GetSHA()),
+			Draft:    pull.GetDraft(),
+		}
+	}
+
+	return changes, nil
+}
+
+// FindChangeByID searches for a change with the given ID.
+func (f *Forge) FindChangeByID(ctx context.Context, id ChangeID) (*FindChangeItem, error) {
+	pull, _, err := f.client.PullRequests.Get(ctx, f.owner, f.repo, int(id))
+	if err != nil {
+		return nil, fmt.Errorf("get pull request: %w", err)
+	}
+
+	return &FindChangeItem{
+		ID:       ChangeID(pull.GetNumber()),
+		URL:      pull.GetHTMLURL(),
+		Subject:  pull.GetTitle(),
+		BaseName: pull.GetBase().GetRef(),
+		HeadHash: git.Hash(pull.GetHead().GetSHA()),
+		Draft:    pull.GetDraft(),
+	}, nil
+}

internal/forge/github/githubtest/shamhub.go

@@ -314,6 +314,7 @@ func (sh *ShamHub) apiHandler() http.Handler {
 	mux.HandleFunc("GET /repos/{owner}/{repo}/pulls", sh.listPullRequests)
 	mux.HandleFunc("POST /repos/{owner}/{repo}/pulls", sh.createPullRequest)
 	mux.HandleFunc("PATCH /repos/{owner}/{repo}/pulls/{number}", sh.updatePullRequest)
+	mux.HandleFunc("GET /repos/{owner}/{repo}/pulls/{number}", sh.getPullRequest)
 	mux.HandleFunc("GET /repos/{owner}/{repo}/pulls/{number}/merge", sh.prIsMerged)
 
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -361,6 +362,51 @@ func (sh *ShamHub) prIsMerged(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
+func (sh *ShamHub) getPullRequest(w http.ResponseWriter, r *http.Request) {
+	owner, repo, numStr := r.PathValue("owner"), r.PathValue("repo"), r.PathValue("number")
+	if owner == "" || repo == "" || numStr == "" {
+		http.Error(w, "owner, repo, and number are required", http.StatusBadRequest)
+		return
+	}
+
+	num, err := strconv.Atoi(numStr)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
+	}
+
+	var (
+		got   shamPR
+		found bool
+	)
+	sh.mu.RLock()
+	for _, pr := range sh.pulls {
+		if pr.Owner == owner && pr.Repo == repo && pr.Number == num {
+			got = pr
+			found = true
+			break
+		}
+	}
+	sh.mu.RUnlock()
+
+	if !found {
+		http.Error(w, "pull request not found", http.StatusNotFound)
+		return
+	}
+
+	ghpr, err := sh.toGitHubPR(&got)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+
+	enc := json.NewEncoder(w)
+	enc.SetIndent("", "  ")
+	if err := enc.Encode(ghpr); err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+	}
+}
+
 func (sh *ShamHub) listPullRequests(w http.ResponseWriter, r *http.Request) {
 	type prMatcher func(*shamPR) bool
 

testdata/script/branch_submit_detect_existing.txt

@@ -0,0 +1,64 @@
+# 'branch submit' should detect a PR created
+# outside of the branch submit command.
+
+as 'Test <[email protected]>'
+at '2024-05-18T13:57:12Z'
+
+# setup
+cd repo
+git init
+git commit --allow-empty -m 'Initial commit'
+
+# set up a fake GitHub remote
+gh-init
+gh-add-remote origin alice/example.git
+git push origin main
+
+# create a new branch and submit it
+git add feature1.txt
+gs bc -m 'Add feature1' feature1
+gs branch submit --fill
+stderr 'Created #'
+
+# forget all state, and re-track the branch
+gs repo init --reset --trunk=main --remote=origin
+gs branch track --base=main feature1
+
+# If we now commit to the branch and then submit,
+# the system should detect that a PR already exists,
+# and update that instead.
+cp $WORK/extra/feature1-update.txt feature1.txt
+git add feature1.txt
+git commit -m 'update feature1'
+
+gs branch submit
+stderr 'feature1: Found existing PR #'
+stderr 'Updated #'
+
+gh-dump-pull
+cmpenvJSON stdout $WORK/golden/update.json
+
+-- repo/feature1.txt --
+Contents of feature1
+
+-- extra/feature1-update.txt --
+New contents of feature1
+
+-- golden/update.json --
+[
+  {
+    "number": 1,
+    "state": "open",
+    "title": "Add feature1",
+    "body": "",
+    "html_url": "$GITHUB_URL/alice/example/pull/1",
+    "head": {
+      "ref": "feature1",
+      "sha": "b805a8b9545d71929cc128fc81b0d86bb2def9ed"
+    },
+    "base": {
+      "ref": "main",
+      "sha": "9df31764fb4252f719c92d53fae05a766f019a17"
+    }
+  }
+]