From 5d7dee3305f6fb84ae69ade1d002e2de4b8b65d1 Mon Sep 17 00:00:00 2001 From: Greg Date: Sun, 25 Oct 2026 18:17:28 -0400 Subject: [PATCH] feat(worker): one draft per game; a rebuild replaces the earlier one The draft branch was named after the job. Rerunning a build pushed onto the earlier run's branch, which had review commits, so the push was rejected and the finished build failed at its last step. Separate branches per run would pile up open PRs for the same game instead. A game now has one draft at a time, on draft/. Before pushing, the worker closes the game's open draft PRs (draft/, and draft/- as earlier versions named them) and deletes their branches, then force-pushes draft/ and opens the new PR, which says which PR it replaces. Other games' drafts and anyone's non-draft branches are never touched. Co-Authored-By: Claude Opus 5.5 --- cmd/gbrgames/worker.go | 110 ++++++++++++++++++++++++++++++------ cmd/gbrgames/worker_test.go | 45 +++++++++++++++ 2 files changed, 137 insertions(+), 18 deletions(-) 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) + } + } +}