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/<id>. Before pushing, the worker closes the game's open draft PRs (draft/<id>, and draft/<id>-<job> as earlier versions named them) and deletes their branches, then force-pushes draft/<id> 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 <noreply@anthropic.com>
This commit is contained in:
1 parent
f1d074bb11
commit
5d7dee3305
2 files changed
+137
-18
No files matched your search
+92
-18
@@ -9,9 +9,11 @@ import (
|
|||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
"net/url"
|
||||||
"os"
|
"os"
|
||||||
"os/exec"
|
"os/exec"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"regexp"
|
||||||
"strings"
|
"strings"
|
||||||
"sync"
|
"sync"
|
||||||
"time"
|
"time"
|
||||||
@@ -354,13 +356,18 @@ func (c *workerConfig) progress(jobID, stage string, lines []string) bool {
|
|||||||
return resp.Cancelled
|
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) {
|
func (c *workerConfig) openPR(dir string, job *builderJob, res *buildResult) (string, error) {
|
||||||
short := job.ID
|
branch := draftBranch(res.GameID)
|
||||||
if len(short) > 8 {
|
replaced, err := c.replaceOldDrafts(res.GameID)
|
||||||
short = short[:8]
|
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
|
auth := "http.extraHeader=Authorization: token " + c.botToken
|
||||||
steps := []struct {
|
steps := []struct {
|
||||||
name string
|
name string
|
||||||
@@ -370,7 +377,8 @@ func (c *workerConfig) openPR(dir string, job *builderJob, res *buildResult) (st
|
|||||||
{"add", []string{"add", filepath.ToSlash(res.Dir)}},
|
{"add", []string{"add", filepath.ToSlash(res.Dir)}},
|
||||||
{"commit", []string{"-c", "user.name=" + c.botLogin, "-c", "user.email=" + c.botEmail, "commit", "-q", "-m",
|
{"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)}},
|
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 {
|
for _, s := range steps {
|
||||||
cmd := exec.Command("git", s.args...)
|
cmd := exec.Command("git", s.args...)
|
||||||
@@ -384,28 +392,94 @@ func (c *workerConfig) openPR(dir string, job *builderJob, res *buildResult) (st
|
|||||||
if !res.Passed {
|
if !res.Passed {
|
||||||
title = "WIP: " + title + " (did not boot yet)"
|
title = "WIP: " + title + " (did not boot yet)"
|
||||||
}
|
}
|
||||||
body := map[string]any{"title": title, "head": branch, "base": "main", "body": res.Summary}
|
var pr struct {
|
||||||
payload, _ := json.Marshal(body)
|
HTMLURL string `json:"html_url"`
|
||||||
req, err := http.NewRequest(http.MethodPost, c.gitea+"/api/v1/repos/"+c.repo+"/pulls", bytes.NewReader(payload))
|
}
|
||||||
if err != nil {
|
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 "", 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("Content-Type", "application/json")
|
||||||
req.Header.Set("Authorization", "token "+c.botToken)
|
req.Header.Set("Authorization", "token "+c.botToken)
|
||||||
resp, err := (&http.Client{Timeout: 30 * time.Second}).Do(req)
|
resp, err := (&http.Client{Timeout: 30 * time.Second}).Do(req)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", err
|
return err
|
||||||
}
|
}
|
||||||
defer resp.Body.Close()
|
defer resp.Body.Close()
|
||||||
var pr struct {
|
raw, _ := io.ReadAll(resp.Body)
|
||||||
HTMLURL string `json:"html_url"`
|
|
||||||
Message string `json:"message"`
|
|
||||||
}
|
|
||||||
json.NewDecoder(resp.Body).Decode(&pr)
|
|
||||||
if resp.StatusCode >= 300 {
|
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 {
|
func readJSON(p string, out any) error {
|
||||||
|
|||||||
@@ -68,3 +68,48 @@ func TestCheckBotWithRepositoryOnlyToken(t *testing.T) {
|
|||||||
t.Errorf("identity = %s <%s>", c.botLogin, c.botEmail)
|
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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in new issue
Block a user