diff --git a/cmd/gbrgames/worker.go b/cmd/gbrgames/worker.go index c8a62fd..5b3a2c2 100644 --- a/cmd/gbrgames/worker.go +++ b/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/, and draft/- 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 { diff --git a/cmd/gbrgames/worker_test.go b/cmd/gbrgames/worker_test.go index 895d5fe..408b8cb 100644 --- a/cmd/gbrgames/worker_test.go +++ b/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) + } + } +}