From 5d1b07bd3e2829427ffafc48abd7c5bab6c4036c Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 9 Nov 2026 09:18:44 +0100 Subject: [PATCH] The forge bridge answers the merged message of a merge made on the forge, not through the gate, by reading that pull request from the forge (issue 1966) Claude-Session: https://claude.ai/code/session_01NcbgfFSnn24M3GcqttoU7i --- .../mesh-delivery-forge-bridge-tools/main.go | 65 +++++++++++++------ .../main_test.go | 39 ++++++++++- .../internal/bridge/forgemade.go | 55 ++++++++++++++++ .../mesh-delivery-indexer/bridgemessage.go | 14 ++-- .../internal/index/candidates.go | 4 +- .../internal/index/pending.go | 4 +- .../internal/index/pending_test.go | 6 +- 7 files changed, 153 insertions(+), 34 deletions(-) create mode 100644 modules/mesh-delivery-forge-bridge/internal/bridge/forgemade.go diff --git a/modules/mesh-delivery-forge-bridge/cmd/mesh-delivery-forge-bridge-tools/main.go b/modules/mesh-delivery-forge-bridge/cmd/mesh-delivery-forge-bridge-tools/main.go index 75346e29..0309cf12 100644 --- a/modules/mesh-delivery-forge-bridge/cmd/mesh-delivery-forge-bridge-tools/main.go +++ b/modules/mesh-delivery-forge-bridge/cmd/mesh-delivery-forge-bridge-tools/main.go @@ -43,7 +43,7 @@ const ( type asker func(key string, value []byte) (groups.EndAsk, error) // Tools are the bridge's tools: groupsRead reads the groups the gate holds with what the bridge kept of them. -func Tools(ask asker, now func() time.Time, groupsRead func() ([]groups.Row, []string, error), merged func(repository, merge string) (json.RawMessage, error), api bridge.API) []stdio.Tool { +func Tools(ask asker, now func() time.Time, groupsRead func() ([]groups.Row, []string, error), merged func(repository, merge string, number int) (json.RawMessage, bool, error), api bridge.API) []stdio.Tool { str := func(args map[string]any, k string) string { s, _ := args[k].(string); return strings.TrimSpace(s) } return []stdio.Tool{ {Name: bridge.TagsTool, @@ -62,16 +62,25 @@ func Tools(ask asker, now func() time.Time, groupsRead func() ([]groups.Row, []s "and lock files of its module directories) and from the gate's merges state (the pull request's number, head and " + "group). For the indexer, which asks it when it takes in a held merge it does not hold in memory (a restart, a new " + "build), and the ask-again a lost message lacked. About 0.5 MB for a repository of many modules. Answers {message} " + - "with the forge message whole, or why it cannot: the gate holds no merge with that commit, or the forge did not answer.", + "with the forge message whole, or why it cannot: neither the gate nor the forge gives a merged pull request with that commit, or the forge did not answer. " + + "A merge made on the forge (a recovery merge, the operator's own) has no merge in the gate: the bridge reads that pull request from the forge, by number when given, and says made_on_forge.", Input: map[string]any{"type": "object", "required": []string{"repository", "merge"}, "properties": map[string]any{ "repository": map[string]any{"type": "string", "description": "the repository, owner/name, as the forge names it"}, - "merge": map[string]any{"type": "string", "description": "the merge commit, 40 hex digits"}}}, + "merge": map[string]any{"type": "string", "description": "the merge commit, 40 hex digits"}, + "number": map[string]any{"type": "integer", "description": "the pull request's number, when known: for a merge made on the forge the bridge reads that pull request and checks its merge commit"}}}, Run: func(args map[string]any) (any, error) { - raw, err := merged(str(args, "repository"), str(args, "merge")) + number, _ := args["number"].(float64) + raw, onForge, err := merged(str(args, "repository"), str(args, "merge"), int(number)) if err != nil { return nil, err } - return map[string]any{"message": raw}, nil + out := map[string]any{"message": raw} + if onForge { + out["made_on_forge"] = true + out["note"] = "the merge was made on the forge, not through the gate: its pull request was read from the forge" + fmt.Fprintf(os.Stderr, "[mesh-delivery-forge-bridge-tools] merged message of %s at %s answered from the forge: the merge was made on the forge, not through the gate\n", str(args, "repository"), str(args, "merge")) + } + return out, nil }}, {Name: GroupsTool, Description: "The delivery groups the gate holds (more than one member), each with its members' merge state and whether each " + @@ -124,10 +133,15 @@ func Tools(ask asker, now func() time.Time, groupsRead func() ([]groups.Row, []s // MergedOf is the merged message of the merge of repository at commit merge, as the bridge writes it: the pull request's // number, head and group from the gate's merges (raw, by key), the rest from the forge through trees and files // (bridge.MergedMessage), and, for a member of a delivery group, the group and its members as the process sets them. -func MergedOf(raws map[string][]byte, repository, merge string, trees bridge.Trees, files bridge.Files) (json.RawMessage, error) { +// +// When the gate holds no merge with that commit, the merge was made on the forge (a recovery merge, the operator's own): +// the pull request is read from the forge through api (by number when given, else by the merge commit), the message is +// built the same way, and madeOnForge is true, for the answer and its log line to say. It is a lone pull request: no +// group is set. +func MergedOf(raws map[string][]byte, repository, merge string, number int, trees bridge.Trees, files bridge.Files, api bridge.API) (out json.RawMessage, madeOnForge bool, err error) { owner, repo, ok := strings.Cut(repository, "/") if !ok || owner == "" || repo == "" { - return nil, fmt.Errorf("%q is not owner/name", repository) + return nil, false, fmt.Errorf("%q is not owner/name", repository) } for _, key := range slices.Sorted(maps.Keys(raws)) { m, err := meshdeliveryclient.DecodeMerge(raws[key]) @@ -143,19 +157,30 @@ func MergedOf(raws map[string][]byte, repository, merge string, trees bridge.Tre p := m.Pulls[at] msg, err := bridge.MergedMessage(owner, repo, int(p.Number), m.Group, p.Head, merge, trees, files) if err != nil { - return nil, fmt.Errorf("the merged message of %s at %s cannot be built: %w", repository, merge, err) + return nil, false, fmt.Errorf("the merged message of %s at %s cannot be built: %w", repository, merge, err) } f, err := forgemsg.DecodeForge(msg.Data) if err != nil { - return nil, err + return nil, false, err } if _, approved := m.Approved[meshdeliveryclient.ApprovedKey(f.Repository, f.Number)]; approved && len(m.Approved) >= 2 { f.Group, f.Members = m.Group, groups.MembersOf(m) } - out, err := forgemsg.EncodeForge(f) - return out, err + out, err = forgemsg.EncodeForge(f) + return out, false, err } - return nil, fmt.Errorf("the gate's merges hold no merge of %s with the commit %s, so its pull request is not known", repository, merge) + if api == nil { + return nil, false, fmt.Errorf("the gate's merges hold no merge of %s with the commit %s, so its pull request is not known", repository, merge) + } + fp, err := bridge.ForgeMade(api, owner, repo, number, merge) + if err != nil { + return nil, false, fmt.Errorf("the gate's merges hold no merge of %s with the commit %s, and the forge does not give it as a merged pull request either: %w", repository, merge, err) + } + msg, err := bridge.MergedMessage(owner, repo, fp.Number, fp.Branch, fp.Head, merge, trees, files) + if err != nil { + return nil, false, fmt.Errorf("the merged message of %s at %s (a merge made on the forge) cannot be built: %w", repository, merge, err) + } + return msg.Data, true, nil } // read is the groups the gate holds with what the bridge kept of them: their ends in performed, and the last ask of @@ -257,14 +282,19 @@ func askCaller(_ context.Context, key string, body any) (json.RawMessage, error) return stdio.Ask(key, body) } +// forgeAPI is one GET of the forge's API through the forge module's gitea_api, the path the bridge reads the forge by. +func forgeAPI(path string) (json.RawMessage, error) { + return stdio.Ask("gitea.gitea_api", map[string]any{"path": path, "method": "GET"}) +} + func main() { merges, performed, asks := stdio.State(gateMergesState), stdio.State(groups.PerformedState), stdio.State(groups.EndAsksState) tools := Tools(askEnd(asks, 25*time.Second), time.Now, func() ([]groups.Row, []string, error) { return read(merges, performed, asks) }, - func(repository, merge string) (json.RawMessage, error) { + func(repository, merge string, number int) (json.RawMessage, bool, error) { keys, err := merges.Keys() if err != nil { - return nil, fmt.Errorf("the gate's merges cannot be listed: %w", err) + return nil, false, fmt.Errorf("the gate's merges cannot be listed: %w", err) } raws := map[string][]byte{} for _, k := range keys { @@ -273,11 +303,8 @@ func main() { } } ctx := context.Background() - return MergedOf(raws, repository, merge, bridge.TreesVia(ctx, askCaller), bridge.FilesVia(ctx, askCaller)) - }, - func(path string) (json.RawMessage, error) { - return stdio.Ask("gitea.gitea_api", map[string]any{"path": path, "method": "GET"}) - }) + return MergedOf(raws, repository, merge, number, bridge.TreesVia(ctx, askCaller), bridge.FilesVia(ctx, askCaller), forgeAPI) + }, forgeAPI) if err := stdio.Serve("", tools); err != nil { fmt.Fprintf(os.Stderr, "[mesh-delivery-forge-bridge-tools] %v\n", err) os.Exit(1) 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 3bcd2e10..b77195f9 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 @@ -157,6 +157,9 @@ func TestTheBridgeToolsReadTheGroupsAndAskTheEndAgain(t *testing.T) { raws map[string][]byte repository string merge string + number int + api bridge.API + onForge bool want forgemsg.Forge fail string }{ @@ -167,9 +170,23 @@ func TestTheBridgeToolsReadTheGroupsAndAskTheEndAgain(t *testing.T) { return f }()}, {name: "a merge the gate does not hold", raws: map[string][]byte{"solo": lone, "bad": []byte("{")}, repository: "novox/a", merge: c2, fail: "hold no merge"}, + // Issue 1966: a merge made on the forge (a recovery merge) has no merge in the gate; the bridge reads the pull request + // from the forge, by number or by the merge commit, and checks its merge commit. + {name: "a merge made on the forge, by number", raws: map[string][]byte{"solo": lone}, repository: "novox/a", merge: c2, number: 9, onForge: true, + api: forgeAnswers(t, "/repos/novox/a/pulls/9", true, "fix/x", h2, c2, "main"), want: published("a", 9, h2, "fix/x", c2)}, + {name: "a merge made on the forge, by the merge commit", raws: map[string][]byte{}, repository: "novox/a", merge: c2, onForge: true, + api: forgeAnswers(t, "/repos/novox/a/commits/"+c2+"/pull", true, "fix/x", h2, c2, "main"), want: published("a", 9, h2, "fix/x", c2)}, + {name: "a forge pull request merged at another commit", raws: map[string][]byte{}, repository: "novox/a", merge: c2, number: 9, + api: forgeAnswers(t, "/repos/novox/a/pulls/9", true, "fix/x", h2, c1, "main"), fail: "not at " + c2}, + {name: "a forge pull request not merged", raws: map[string][]byte{}, repository: "novox/a", merge: c2, number: 9, + 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 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"}, } { t.Run(c.name, func(t *testing.T) { - raw, err := MergedOf(c.raws, c.repository, c.merge, trees, files) + raw, onForge, err := MergedOf(c.raws, c.repository, c.merge, c.number, trees, files, c.api) if c.fail != "" { if err == nil || !strings.Contains(err.Error(), c.fail) { t.Fatalf("err %v, want %q", err, c.fail) @@ -177,15 +194,31 @@ func TestTheBridgeToolsReadTheGroupsAndAskTheEndAgain(t *testing.T) { return } want, _ := forgemsg.EncodeForge(c.want) + if err == nil && onForge != c.onForge { + t.Fatalf("made on the forge %v, want %v", onForge, c.onForge) + } if err != nil || !bytes.Equal(raw, want) { t.Fatalf("err %v, answer\n%s\nwant the published message\n%s", err, raw, want) } }) } - if _, err := MergedOf(map[string][]byte{"solo": lone}, "novox/a", c1, func(string, string, string) (string, map[string]string, error) { + if _, _, err := MergedOf(map[string][]byte{"solo": lone}, "novox/a", c1, 0, func(string, string, string) (string, map[string]string, error) { return "", nil, errors.New("the forge does not answer") - }, files); err == nil || !strings.Contains(err.Error(), "does not answer") { + }, files, nil); err == nil || !strings.Contains(err.Error(), "does not answer") { t.Errorf("a forge that does not answer: %v", err) } }) } + +// forgeAnswers is the forge's gitea_api for one GET path: that pull request as the forge says it; any other path is not found. +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}}) + return func(p string) (json.RawMessage, error) { + if p != path { + return nil, errors.New("the forge has no " + p) + } + return body, nil + } +} diff --git a/modules/mesh-delivery-forge-bridge/internal/bridge/forgemade.go b/modules/mesh-delivery-forge-bridge/internal/bridge/forgemade.go new file mode 100644 index 00000000..c5ed8632 --- /dev/null +++ b/modules/mesh-delivery-forge-bridge/internal/bridge/forgemade.go @@ -0,0 +1,55 @@ +package bridge + +import ( + "encoding/json" + "fmt" + "net/url" +) + +// ForgePull is a pull request as the forge itself says it: what the bridge needs of a merge made on the forge, not +// through the gate (a recovery merge or the operator's own, which the gate holds no merge of; issue 1966). +type ForgePull struct { + Number int + Branch string // the pull request's head branch + Head string // the head commit + Merge string // the merge commit into the trunk +} + +// ForgeMade reads, through the forge module's gitea_api (the path the bridge reads the forge by), the merged pull +// request whose merge commit is merge: by number when number is given (the pull request's merge commit must then equal +// merge), else by the merge commit (the forge's commit-to-pull-request lookup). A pull request not merged, merged into +// another branch than the trunk, or merged at another commit is refused. +func ForgeMade(api API, owner, repo string, number int, merge string) (ForgePull, error) { + path := fmt.Sprintf("/repos/%s/%s/commits/%s/pull", url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(merge)) + if number > 0 { + path = fmt.Sprintf("/repos/%s/%s/pulls/%d", url.PathEscape(owner), url.PathEscape(repo), number) + } + raw, err := api(path) + if err != nil { + return ForgePull{}, fmt.Errorf("the forge did not answer %s: %w", path, err) + } + var p struct { + Number int `json:"number"` + Merged bool `json:"merged"` + MergeCommitSHA string `json:"merge_commit_sha"` + Head struct { + Ref string `json:"ref"` + SHA string `json:"sha"` + } `json:"head"` + Base struct { + Ref string `json:"ref"` + } `json:"base"` + } + if err := json.Unmarshal(raw, &p); err != nil { + return ForgePull{}, fmt.Errorf("the forge's answer for %s is no pull request: %w", path, err) + } + switch { + case !p.Merged: + return ForgePull{}, fmt.Errorf("the forge says the pull request %s/%s#%d is not merged", owner, repo, p.Number) + case p.MergeCommitSHA != merge: + return ForgePull{}, fmt.Errorf("the forge says %s/%s#%d was merged at %s, not at %s", owner, repo, p.Number, p.MergeCommitSHA, merge) + 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 +} diff --git a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/bridgemessage.go b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/bridgemessage.go index a2e3212c..1bf9196e 100644 --- a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/bridgemessage.go +++ b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/bridgemessage.go @@ -8,16 +8,20 @@ import ( "mesh-delivery-indexer/internal/index" ) -// mergedTool is the forge bridge's tool that answers a merge's merged message again (issue 1585): arguments repository -// and merge, answer {message}, the message whole as the bridge writes it on FORGE. The indexer's manifest lists it in +// mergedTool is the forge bridge's tool that answers a merge's merged message again (issue 1585): arguments repository, +// merge and, when the indexer knows it, number (the bridge then reads a merge made on the forge itself), answer {message}, the message whole as the bridge writes it on FORGE. The indexer's manifest lists it in // invokes, which is the grant to call it. const mergedTool = index.ForgeBridgeModule + ".bridge_merged_message" // messageVia asks the forge bridge for a merge's merged message, for a held merge the indexer does not hold in memory (a // restart, a new build). An error is the bridge's own words. -func messageVia(call toolCaller) func(ctx context.Context, repository, merge string) ([]byte, error) { - return func(_ context.Context, repository, merge string) ([]byte, error) { - raw, err := call(mergedTool, map[string]string{"repository": repository, "merge": merge}) +func messageVia(call toolCaller) func(ctx context.Context, repository, merge string, number int) ([]byte, error) { + return func(_ context.Context, repository, merge string, number int) ([]byte, error) { + ask := map[string]any{"repository": repository, "merge": merge} + if number > 0 { + ask["number"] = number + } + raw, err := call(mergedTool, ask) if err != nil { return nil, err } diff --git a/modules/mesh-delivery-indexer/internal/index/candidates.go b/modules/mesh-delivery-indexer/internal/index/candidates.go index 6eaa3cb7..ad3613ac 100644 --- a/modules/mesh-delivery-indexer/internal/index/candidates.go +++ b/modules/mesh-delivery-indexer/internal/index/candidates.go @@ -169,9 +169,9 @@ type Indexer struct { // OwnBuildState keeps which build this process knows it runs, or since when and why not (FixOwnBuild); nil keeps none. OwnBuildState Store judgePending atomic.Bool // the start's judge pass failed: judged again at the next message (verdicts.go) - // Message asks the forge bridge for the merged message of a merge, repository and merge commit in, the message whole out (a + // Message asks the forge bridge for the merged message of a merge, repository, merge commit and pull request number in (the number lets the bridge read a merge made on the forge, not through the gate), the message whole out (a // pending value keeps a reference, not a copy: issue 1585); an error is the bridge's words. Nil asks none. - Message func(ctx context.Context, repository, merge string) ([]byte, error) + Message func(ctx context.Context, repository, merge string, number int) ([]byte, error) heldMu sync.Mutex held map[uint64]forgemsg.Forge // the merged messages Arrive was handed, by stream sequence, until their merge is taken in or dropped // Pending is the state of merges held for the indexer their revision names (pending.go, novox/hq ADR 0633 ยง1); nil diff --git a/modules/mesh-delivery-indexer/internal/index/pending.go b/modules/mesh-delivery-indexer/internal/index/pending.go index cd2a6aac..4b097d04 100644 --- a/modules/mesh-delivery-indexer/internal/index/pending.go +++ b/modules/mesh-delivery-indexer/internal/index/pending.go @@ -225,7 +225,7 @@ func (ix *Indexer) load(ctx context.Context, e PendingEntry) (PendingEntry, erro if ix.Message == nil { return e, fmt.Errorf("the forge bridge is not asked by this indexer, so %s (%s#%d) cannot be read", e.Key, e.Forge.Repository, e.Forge.Number) } - data, err := ix.Message(ctx, e.Forge.Repository, e.Forge.Merge) + data, err := ix.Message(ctx, e.Forge.Repository, e.Forge.Merge, e.Forge.Number) if err != nil { return e, fmt.Errorf("the forge bridge could not answer for the merge of %s#%d at %s: %w", e.Forge.Repository, e.Forge.Number, e.Forge.Merge, err) } @@ -1292,7 +1292,7 @@ func (ix *Indexer) TakeInMerge(ctx context.Context, repository, merge string, se r.Answer = "this indexer does not ask the forge bridge" return r } - data, err := ix.Message(ctx, repository, merge) + data, err := ix.Message(ctx, repository, merge, 0) if err != nil { r.Answer = fmt.Sprintf("the forge bridge could not answer for the merge of %s at %s: %v", repository, merge, err) return r diff --git a/modules/mesh-delivery-indexer/internal/index/pending_test.go b/modules/mesh-delivery-indexer/internal/index/pending_test.go index b971a30c..29807aab 100644 --- a/modules/mesh-delivery-indexer/internal/index/pending_test.go +++ b/modules/mesh-delivery-indexer/internal/index/pending_test.go @@ -46,7 +46,7 @@ func newTakeInBed(t *testing.T) *takeInBed { ix.Verdicts = newMem() b.pending.maxValue = buscompose.StateMaxValueBytes ix.Pending = b.pending - ix.Message = func(_ context.Context, repository, merge string) ([]byte, error) { + ix.Message = func(_ context.Context, repository, merge string, number int) ([]byte, error) { if data, ok := b.bridge[repository+"@"+merge]; ok { return data, nil } @@ -511,7 +511,7 @@ func TestTakeIn(t *testing.T) { // 1585/T3-T13 (ADR 0633) b = held() answers := b.ix.Message b.ix.held = nil - b.ix.Message = func(context.Context, string, string) ([]byte, error) { return nil, errors.New("timed out") } + b.ix.Message = func(context.Context, string, string, int) ([]byte, error) { return nil, errors.New("timed out") } if _, _, err := b.ix.TakeInAtStart(ctx); err != nil || len(b.keys()) != 2 { t.Fatalf("a lost start: err %v, pending %v", err, b.keys()) } @@ -669,7 +669,7 @@ func TestTakeIn(t *testing.T) { // 1585/T3-T13 (ADR 0633) avahi0 := b.fp(t, "avahi") // The forge bridge is not asked at all while the merges are in memory: a 300 KiB merge, over the bucket's cap whole, // is held by reference and a lone one taken in. - bridgeDown := func(context.Context, string, string) ([]byte, error) { + bridgeDown := func(context.Context, string, string, int) ([]byte, error) { return nil, errors.New("the bridge does not answer") } answers := b.ix.Message