Merge pull request 'feat(worker): one draft per game; a rebuild replaces the earlier one' (#15) from fix/worker-branch-per-run into main

Reviewed-on: #15

cmd/gbrgames/worker.go

@@ -9,9 +9,11 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"sync"
 	"time"
@@ -354,13 +356,18 @@ func (c *workerConfig) progress(jobID, stage string, lines []string) bool {
 	return resp.Cancelled
 }
 
-// openPR commits the new game folder to its own branch as the bot and opens a pull request to main.
+// openPR commits the new game folder to the game's draft branch as the bot and opens a pull request to
+// main. A game has one draft at a time: an earlier draft's PR is closed and its branch deleted first.
 func (c *workerConfig) openPR(dir string, job *builderJob, res *buildResult) (string, error) {
-	short := job.ID
-	if len(short) > 8 {
-		short = short[:8]
+	branch := draftBranch(res.GameID)
+	replaced, err := c.replaceOldDrafts(res.GameID)
+	if err != nil {
+		return "", fmt.Errorf("replacing the earlier draft: %w", err)
+	}
+	summary := res.Summary
+	for _, n := range replaced {
+		summary += fmt.Sprintf("\nReplaces #%d (closed, branch deleted).\n", n)
 	}
-	branch := "draft/" + res.GameID + "-" + short
 	auth := "http.extraHeader=Authorization: token " + c.botToken
 	steps := []struct {
 		name string
@@ -370,7 +377,8 @@ func (c *workerConfig) openPR(dir string, job *builderJob, res *buildResult) (st
 		{"add", []string{"add", filepath.ToSlash(res.Dir)}},
 		{"commit", []string{"-c", "user.name=" + c.botLogin, "-c", "user.email=" + c.botEmail, "commit", "-q", "-m",
 			fmt.Sprintf("feat(games): draft %s\n\nDrafted and boot-tested by the GBR builder (job %s).", job.GameName, job.ID)}},
-		{"push", []string{"-c", auth, "push", "-q", "origin", branch}},
+		// Forced: the branch is the bot's own, and whatever was on it was the draft being replaced
+		{"push", []string{"-c", auth, "push", "-q", "--force", "origin", branch}},
 	}
 	for _, s := range steps {
 		cmd := exec.Command("git", s.args...)
@@ -384,28 +392,94 @@ func (c *workerConfig) openPR(dir string, job *builderJob, res *buildResult) (st
 	if !res.Passed {
 		title = "WIP: " + title + " (did not boot yet)"
 	}
-	body := map[string]any{"title": title, "head": branch, "base": "main", "body": res.Summary}
-	payload, _ := json.Marshal(body)
-	req, err := http.NewRequest(http.MethodPost, c.gitea+"/api/v1/repos/"+c.repo+"/pulls", bytes.NewReader(payload))
-	if err != nil {
+	var pr struct {
+		HTMLURL string `json:"html_url"`
+	}
+	body := map[string]any{"title": title, "head": branch, "base": "main", "body": summary}
+	if err := c.giteaCall(http.MethodPost, "/repos/"+c.repo+"/pulls", body, &pr); err != nil {
 		return "", err
 	}
+	return pr.HTMLURL, nil
+}
+
+// replaceOldDrafts closes the game's open draft PRs and deletes their branches, so a rebuild leaves one
+// draft to review instead of a pile. Only draft/ branches for this game are touched, never anyone's other
+// work. Returns the numbers of the PRs it closed.
+func (c *workerConfig) replaceOldDrafts(gameID string) ([]int, error) {
+	var pulls []struct {
+		Number int `json:"number"`
+		Head   struct {
+			Ref string `json:"ref"`
+		} `json:"head"`
+	}
+	if err := c.giteaCall(http.MethodGet, "/repos/"+c.repo+"/pulls?state=open&limit=50", nil, &pulls); err != nil {
+		return nil, err
+	}
+	branches := map[string]bool{draftBranch(gameID): true}
+	var closed []int
+	for _, p := range pulls {
+		if !isDraftBranchOf(p.Head.Ref, gameID) {
+			continue
+		}
+		if err := c.giteaCall(http.MethodPatch, fmt.Sprintf("/repos/%s/pulls/%d", c.repo, p.Number), map[string]string{"state": "closed"}, nil); err != nil {
+			return closed, err
+		}
+		fmt.Printf("[worker] closed #%d, the earlier draft of %s\n", p.Number, gameID)
+		closed = append(closed, p.Number)
+		branches[p.Head.Ref] = true
+	}
+	for b := range branches {
+		// Gone already is fine: not every game has had a draft
+		if err := c.giteaCall(http.MethodDelete, "/repos/"+c.repo+"/branches/"+url.PathEscape(b), nil, nil); err != nil && !strings.HasPrefix(err.Error(), "HTTP 404") {
+			return closed, fmt.Errorf("deleting branch %s: %w", b, err)
+		}
+	}
+	return closed, nil
+}
+
+func draftBranch(gameID string) string {
+	return "draft/" + gameID
+}
+
+// isDraftBranchOf matches draft/<id>, and draft/<id>-<job> as the worker named them before.
+func isDraftBranchOf(ref, gameID string) bool {
+	d := draftBranch(gameID)
+	if ref == d {
+		return true
+	}
+	rest, ok := strings.CutPrefix(ref, d+"-")
+	return ok && regexp.MustCompile(`^[0-9a-f]{8}$`).MatchString(rest)
+}
+
+// giteaCall sends one API call as the bot; out may be nil.
+func (c *workerConfig) giteaCall(method, p string, body, out any) error {
+	var rd io.Reader
+	if body != nil {
+		payload, err := json.Marshal(body)
+		if err != nil {
+			return err
+		}
+		rd = bytes.NewReader(payload)
+	}
+	req, err := http.NewRequest(method, c.gitea+"/api/v1"+p, rd)
+	if err != nil {
+		return err
+	}
 	req.Header.Set("Content-Type", "application/json")
 	req.Header.Set("Authorization", "token "+c.botToken)
 	resp, err := (&http.Client{Timeout: 30 * time.Second}).Do(req)
 	if err != nil {
-		return "", err
+		return err
 	}
 	defer resp.Body.Close()
-	var pr struct {
-		HTMLURL string `json:"html_url"`
-		Message string `json:"message"`
-	}
-	json.NewDecoder(resp.Body).Decode(&pr)
+	raw, _ := io.ReadAll(resp.Body)
 	if resp.StatusCode >= 300 {
-		return "", fmt.Errorf("HTTP %d: %s", resp.StatusCode, pr.Message)
+		return fmt.Errorf("HTTP %d: %.200s", resp.StatusCode, raw)
 	}
-	return pr.HTMLURL, nil
+	if out != nil && len(bytes.TrimSpace(raw)) > 0 {
+		return json.Unmarshal(raw, out)
+	}
+	return nil
 }
 
 func readJSON(p string, out any) error {

cmd/gbrgames/worker_test.go

@@ -68,3 +68,48 @@ func TestCheckBotWithRepositoryOnlyToken(t *testing.T) {
 		t.Errorf("identity = %s <%s>", c.botLogin, c.botEmail)
 	}
 }
+
+func TestReplaceOldDraftsClosesOnlyThisGamesDrafts(t *testing.T) {
+	var calls []string
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		calls = append(calls, r.Method+" "+r.URL.Path)
+		switch {
+		case r.Method == http.MethodGet && r.URL.Path == "/api/v1/repos/org/games/pulls":
+			w.Write([]byte(`[
+				{"number": 13, "head": {"ref": "draft/example-4b174b2d"}},
+				{"number": 20, "head": {"ref": "draft/example"}},
+				{"number": 21, "head": {"ref": "draft/example-other"}},
+				{"number": 22, "head": {"ref": "draft/other-4b174b2d"}},
+				{"number": 23, "head": {"ref": "fix/example-ports"}}
+			]`))
+		case r.Method == http.MethodDelete && r.URL.Path == "/api/v1/repos/org/games/branches/draft/example-4b174b2d":
+			w.WriteHeader(http.StatusNoContent)
+		case r.Method == http.MethodDelete:
+			w.WriteHeader(http.StatusNotFound) // already gone is fine
+		default:
+			w.Write([]byte(`{}`))
+		}
+	}))
+	defer srv.Close()
+
+	c := &workerConfig{gitea: srv.URL, repo: "org/games", botToken: "secret"}
+	closed, err := c.replaceOldDrafts("example")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(closed) != 2 || closed[0] != 13 || closed[1] != 20 {
+		t.Errorf("closed %v, want [13 20]: not another game's draft, not draft/example-other, not someone's fix branch", closed)
+	}
+	got := strings.Join(calls, "\n")
+	for _, want := range []string{"PATCH /api/v1/repos/org/games/pulls/13", "PATCH /api/v1/repos/org/games/pulls/20",
+		"DELETE /api/v1/repos/org/games/branches/draft/example-4b174b2d", "DELETE /api/v1/repos/org/games/branches/draft/example"} {
+		if !strings.Contains(got, want) {
+			t.Errorf("missing %q in:\n%s", want, got)
+		}
+	}
+	for _, unwanted := range []string{"pulls/21", "pulls/22", "pulls/23"} {
+		if strings.Contains(got, unwanted) {
+			t.Errorf("touched %s:\n%s", unwanted, got)
+		}
+	}
+}