diff --git a/modules/mesh-delivery-forge-bridge/cmd/mesh-delivery-forge-bridge-tools/main_test.go b/modules/mesh-delivery-forge-bridge/cmd/mesh-delivery-forge-bridge-tools/main_test.go index b77195f9..98bbd789 100644 --- a/modules/mesh-delivery-forge-bridge/cmd/mesh-delivery-forge-bridge-tools/main_test.go +++ b/modules/mesh-delivery-forge-bridge/cmd/mesh-delivery-forge-bridge-tools/main_test.go @@ -182,6 +182,8 @@ func TestTheBridgeToolsReadTheGroupsAndAskTheEndAgain(t *testing.T) { api: forgeAnswers(t, "/repos/novox/a/pulls/9", false, "fix/x", h2, "", "main"), fail: "not merged"}, {name: "a forge pull request merged into another branch", raws: map[string][]byte{}, repository: "novox/a", merge: c2, number: 9, api: forgeAnswers(t, "/repos/novox/a/pulls/9", true, "fix/x", h2, c2, "dev"), fail: "not the trunk"}, + {name: "a forge pull request with no head branch", raws: map[string][]byte{}, repository: "novox/a", merge: c2, number: 9, + api: forgeAnswers(t, "/repos/novox/a/pulls/9", true, "refs/pull/9/head", h2, c2, "main"), fail: "no head branch"}, {name: "a forge that does not answer", raws: map[string][]byte{}, repository: "novox/a", merge: c2, api: func(string) (json.RawMessage, error) { return nil, errors.New("the forge is down") }, fail: "the forge is down"}, } { @@ -214,7 +216,7 @@ func TestTheBridgeToolsReadTheGroupsAndAskTheEndAgain(t *testing.T) { func forgeAnswers(t *testing.T, path string, merged bool, branch, head, merge, base string) bridge.API { t.Helper() body, _ := json.Marshal(map[string]any{"number": 9, "merged": merged, "merge_commit_sha": merge, - "head": map[string]any{"ref": branch, "sha": head}, "base": map[string]any{"ref": base}}) + "head": map[string]any{"ref": "refs/pull/9/head", "label": branch, "sha": head}, "base": map[string]any{"ref": base}}) return func(p string) (json.RawMessage, error) { if p != path { return nil, errors.New("the forge has no " + p) diff --git a/modules/mesh-delivery-forge-bridge/internal/bridge/forgemade.go b/modules/mesh-delivery-forge-bridge/internal/bridge/forgemade.go index c5ed8632..d09e2929 100644 --- a/modules/mesh-delivery-forge-bridge/internal/bridge/forgemade.go +++ b/modules/mesh-delivery-forge-bridge/internal/bridge/forgemade.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "net/url" + "strings" ) // ForgePull is a pull request as the forge itself says it: what the bridge needs of a merge made on the forge, not @@ -33,8 +34,8 @@ func ForgeMade(api API, owner, repo string, number int, merge string) (ForgePull Merged bool `json:"merged"` MergeCommitSHA string `json:"merge_commit_sha"` Head struct { - Ref string `json:"ref"` - SHA string `json:"sha"` + Label string `json:"label"` + SHA string `json:"sha"` } `json:"head"` Base struct { Ref string `json:"ref"` @@ -51,5 +52,9 @@ func ForgeMade(api API, owner, repo string, number int, merge string) (ForgePull case p.Base.Ref != Trunk: return ForgePull{}, fmt.Errorf("the forge says %s/%s#%d was merged into %s, not the trunk %s", owner, repo, p.Number, p.Base.Ref, Trunk) } - return ForgePull{Number: p.Number, Branch: p.Head.Ref, Head: p.Head.SHA, Merge: p.MergeCommitSHA}, nil + // For a merged pull request the forge's head.ref is "refs/pull/N/head"; the branch is in head.label (same-repository pull requests). + if p.Head.Label == "" || strings.HasPrefix(p.Head.Label, "refs/") { + return ForgePull{}, fmt.Errorf("the forge gives no head branch for %s/%s#%d (label %q)", owner, repo, p.Number, p.Head.Label) + } + return ForgePull{Number: p.Number, Branch: p.Head.Label, Head: p.Head.SHA, Merge: p.MergeCommitSHA}, nil } diff --git a/modules/mesh-delivery-indexer/internal/index/pending.go b/modules/mesh-delivery-indexer/internal/index/pending.go index 4b097d04..39a6e79f 100644 --- a/modules/mesh-delivery-indexer/internal/index/pending.go +++ b/modules/mesh-delivery-indexer/internal/index/pending.go @@ -233,7 +233,7 @@ func (ix *Indexer) load(ctx context.Context, e PendingEntry) (PendingEntry, erro if err != nil { return e, fmt.Errorf("the forge bridge's answer for %s is not a forge message: %w", e.Key, err) } - if f.Kind != forgemsg.ForgeMerged || f.Repository != e.Forge.Repository || f.Merge != e.Forge.Merge { + if f.Kind != forgemsg.ForgeMerged || f.Repository != e.Forge.Repository || f.Merge != e.Forge.Merge || (e.Forge.Number > 0 && f.Number != e.Forge.Number) { return e, fmt.Errorf("the forge bridge's answer is not the merge %s#%d at %s that %s refers to", e.Forge.Repository, e.Forge.Number, e.Forge.Merge, e.Key) } e.Forge, e.Light = f, false