Przeglądaj źródła

Deploy the tip of main, not the webhook's merge_commit_sha

Gogs sets merge_commit_sha to the head branch's commit
(database/pull.go: BranchCommitID(pr.HeadBranch)), not the merge commit.
The first real release deployed 0bfb14e, the branch commit, instead of
the merge f9f3973. The trees matched that time, but with two PRs
branched from the same main, the second release would drop the first
PR's changes.

deployd now always releases origin/main; release fetches first and runs
one at a time, so that's exactly what has been merged. The head commit
goes in the log line for reference. test.sh gains the two-PR case: main
moves on after the work clone exists, and the release must include it
(23 checks). The sandbox also strips deploy tags copied from the source.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude 19 godzin temu
rodzic
commit
b4e5d67deb
4 zmienionych plików z 58 dodań i 22 usunięć
  1. 8 2
      README.md
  2. 13 3
      deployd/main.go
  3. 21 17
      deployd/main_test.go
  4. 16 0
      release/test.sh

+ 8 - 2
README.md

@@ -4,14 +4,20 @@ Apps and scripts built as part of the DevOps pipeline for the monoaf.com
 server.
 
 The main piece is the **deploy pipeline**: when a pull request is merged into
-`main` in Gogs, the exact merge commit is built, deployed, tagged
+`main` in Gogs, the new tip of `main` is built, deployed, tagged
 `deploy/<target>/<UTC time>` and logged.
 
+It deploys `origin/main` rather than the webhook's `merge_commit_sha`,
+because Gogs fills that field with the head branch's commit, not the merge
+commit. Deploying the branch commit would drop any other PRs merged since the
+branch was made. Releases run one at a time and each fetches first, so the
+tip of `main` is always what's merged.
+
 ```
  PR merged in Gogs
    │  webhook (HMAC-signed), 127.0.0.1:9000/deploy
    ▼
- deployd  ── checks signature, merged into main ──▶  release <repo> <sha>
+ deployd  ── checks signature, merged into main ──▶  release <repo> origin/main
                                                      │
               clean checkout of that commit ◀────────┤
               go build  /  rsync static files ◀──────┤

+ 13 - 3
deployd/main.go

@@ -18,12 +18,18 @@ import (
 
 const releaseBin = "/usr/local/bin/release"
 
+// deployRef is what a merge releases: the tip of main after release fetches.
+// Not the payload's merge_commit_sha: Gogs fills that with the head branch's
+// commit (database/pull.go, BranchCommitID(pr.HeadBranch)), which lacks any
+// other PRs merged since the branch was made.
+const deployRef = "origin/main"
+
 type payload struct {
 	Action      string `json:"action"`
 	PullRequest struct {
 		Merged         bool   `json:"merged"`
 		BaseBranch     string `json:"base_branch"`
-		MergeCommitSHA string `json:"merge_commit_sha"`
+		MergeCommitSHA string `json:"merge_commit_sha"` // the head branch's commit; logged only
 	} `json:"pull_request"`
 	Repository struct {
 		FullName string `json:"full_name"`
@@ -64,15 +70,19 @@ func handler(secret []byte, release releaser) http.Handler {
 			return
 		}
 		pr := p.PullRequest
-		if p.Action != "closed" || !pr.Merged || pr.BaseBranch != "main" || pr.MergeCommitSHA == "" || p.Repository.FullName == "" {
+		if p.Action != "closed" || !pr.Merged || pr.BaseBranch != "main" || p.Repository.FullName == "" {
 			w.WriteHeader(http.StatusNoContent)
 			return
 		}
+		by := "PR merged by " + p.Sender.Username
+		if head := pr.MergeCommitSHA; len(head) >= 7 {
+			by += " (head " + head[:7] + ")"
+		}
 		w.WriteHeader(http.StatusAccepted)
 		go func() {
 			mu.Lock()
 			defer mu.Unlock()
-			release(p.Repository.FullName, pr.MergeCommitSHA, "PR merged by "+p.Sender.Username)
+			release(p.Repository.FullName, deployRef, by)
 		}()
 	})
 }

+ 21 - 17
deployd/main_test.go

@@ -21,23 +21,27 @@ func sign(body string) string {
 type call struct{ repo, sha, by string }
 
 func TestHandler(t *testing.T) {
-	merged := `{"action":"closed","pull_request":{"merged":true,"base_branch":"main","merge_commit_sha":"abc123"},"repository":{"full_name":"bajdev/posts"},"sender":{"username":"bajdev"}}`
+	merged := `{"action":"closed","pull_request":{"merged":true,"base_branch":"main","merge_commit_sha":"0bfb14ee4067"},"repository":{"full_name":"bajdev/posts"},"sender":{"username":"bajdev"}}`
+	// Every release is of origin/main, whatever merge_commit_sha says: Gogs
+	// puts the head branch's commit there, not the merge commit.
+	withHead := call{"bajdev/posts", "origin/main", "PR merged by bajdev (head 0bfb14e)"}
 	tests := []struct {
 		name, method, event, body, sig string
 		status                         int
-		released                       bool
+		want                           *call // nil: no release
 	}{
-		{"merged into main", "POST", "pull_request", merged, "", 202, true},
-		{"bad signature", "POST", "pull_request", merged, "deadbeef", 401, false},
-		{"missing signature", "POST", "pull_request", merged, "-", 401, false},
-		{"GET", "GET", "pull_request", merged, "", 405, false},
-		{"push event", "POST", "push", merged, "", 204, false},
-		{"opened", "POST", "pull_request", strings.Replace(merged, `"closed"`, `"opened"`, 1), "", 204, false},
-		{"closed without merge", "POST", "pull_request", strings.Replace(merged, `"merged":true`, `"merged":false`, 1), "", 204, false},
-		{"merged into another branch", "POST", "pull_request", strings.Replace(merged, `"main"`, `"dev"`, 1), "", 204, false},
-		{"no merge commit", "POST", "pull_request", strings.Replace(merged, `"abc123"`, `""`, 1), "", 204, false},
-		{"no repository", "POST", "pull_request", strings.Replace(merged, `"bajdev/posts"`, `""`, 1), "", 204, false},
-		{"not JSON", "POST", "pull_request", "not json", "", 204, false},
+		{"merged into main", "POST", "pull_request", merged, "", 202, &withHead},
+		{"no merge commit sha", "POST", "pull_request", strings.Replace(merged, `"0bfb14ee4067"`, `""`, 1), "", 202,
+			&call{"bajdev/posts", "origin/main", "PR merged by bajdev"}},
+		{"bad signature", "POST", "pull_request", merged, "deadbeef", 401, nil},
+		{"missing signature", "POST", "pull_request", merged, "-", 401, nil},
+		{"GET", "GET", "pull_request", merged, "", 405, nil},
+		{"push event", "POST", "push", merged, "", 204, nil},
+		{"opened", "POST", "pull_request", strings.Replace(merged, `"closed"`, `"opened"`, 1), "", 204, nil},
+		{"closed without merge", "POST", "pull_request", strings.Replace(merged, `"merged":true`, `"merged":false`, 1), "", 204, nil},
+		{"merged into another branch", "POST", "pull_request", strings.Replace(merged, `"main"`, `"dev"`, 1), "", 204, nil},
+		{"no repository", "POST", "pull_request", strings.Replace(merged, `"bajdev/posts"`, `""`, 1), "", 204, nil},
+		{"not JSON", "POST", "pull_request", "not json", "", 204, nil},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -59,14 +63,14 @@ func TestHandler(t *testing.T) {
 			}
 			select {
 			case c := <-calls:
-				if !tt.released {
+				if tt.want == nil {
 					t.Fatalf("unexpected release %+v", c)
 				}
-				if c != (call{"bajdev/posts", "abc123", "PR merged by bajdev"}) {
-					t.Errorf("release called with %+v", c)
+				if c != *tt.want {
+					t.Errorf("release called with %+v, want %+v", c, *tt.want)
 				}
 			case <-time.After(200 * time.Millisecond):
-				if tt.released {
+				if tt.want != nil {
 					t.Fatal("release was not called")
 				}
 			}

+ 16 - 0
release/test.sh

@@ -16,6 +16,8 @@ check() { local what=$1; shift; if "$@"; then ok "$what"; else bad "$what"; fi;
 # Remote: bajdev/posts (go app with content) and bajdev/site (static).
 mkdir -p "$T/remote/bajdev" "$T/apps/posts/content" "$T/vhosts/site.example" "$T/bin"
 git clone --quiet --bare "$src" "$T/remote/bajdev/posts.git"
+# Start from no deploy tags, whatever the source repo already has.
+git -C "$T/remote/bajdev/posts.git" tag -l 'deploy/*' | xargs -r git -C "$T/remote/bajdev/posts.git" tag -d >/dev/null
 git init --quiet -b main "$T/site" && mkdir "$T/site/public" && echo hi > "$T/site/public/index.html"
 git -C "$T/site" add . && git -C "$T/site" -c user.name=t -c user.email=t@t commit --quiet -m init
 git clone --quiet --bare "$T/site" "$T/remote/bajdev/site.git"
@@ -83,5 +85,19 @@ check "failed start exits non-zero" bash -c '! SYSTEMCTL_FAIL=1 "$0" bajdev/post
 check "failed start not tagged"   test "$(git -C "$T/remote/bajdev/posts.git" tag -l 'deploy/posts/*' | wc -l)" -eq "$tags_before"
 check "failed start not logged"   test "$(wc -l < "$T/deploys.log")" -eq "$lines_before"
 
+# 6. What deployd sends: origin/main, after main has moved on since the work
+# clone was made (another PR merged meanwhile). The release must include it.
+up=$T/upstream
+git clone --quiet "$T/remote/bajdev/posts.git" "$up"
+printf -- '---\ntitle: Merged by another PR\n---\n\nHi.\n' > "$up/content/2026-12-01-other-pr.md"
+git -C "$up" add content && git -C "$up" commit --quiet -m "Another PR" && git -C "$up" push --quiet origin HEAD:main
+tip=$(git -C "$T/remote/bajdev/posts.git" rev-parse main)
+sleep 1
+out=$(DEPLOY_BY="PR merged by bajdev (head 0bfb14e)" "$R" bajdev/posts origin/main 2>&1) || bad "release origin/main: $out"
+check "origin/main includes the other PR" test -f "$T/apps/posts/content/2026-12-01-other-pr.md"
+tagN=$(git -C "$T/remote/bajdev/posts.git" tag -l 'deploy/posts/*' | tail -1)
+check "tag is on main's new tip"  test "$(git -C "$T/remote/bajdev/posts.git" rev-parse "$tagN^{commit}")" = "$tip"
+check "log records main's tip"    grep -q "posts $tip $tagN PR merged by bajdev (head 0bfb14e)" "$T/deploys.log"
+
 echo "$pass passed, $fail failed"
 [[ $fail -eq 0 ]]