From ea3eac51bd670577a50886850b4a4c58c2fd3825 Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 27 Oct 2026 02:13:55 +0100 Subject: [PATCH] Carry a catalogue pull request's placement files at its head when it touches placement/, so the indexer can write them as candidates for the nodes to answer before the merge (issue 1126) --- modules/mesh-delivery-forge-bridge/go.mod | 2 +- modules/mesh-delivery-forge-bridge/go.sum | 4 ++ .../internal/bridge/bridge.go | 29 ++++++++++- .../internal/bridge/files_test.go | 49 +++++++++++++++++++ 4 files changed, 82 insertions(+), 2 deletions(-) diff --git a/modules/mesh-delivery-forge-bridge/go.mod b/modules/mesh-delivery-forge-bridge/go.mod index 6d3738c..fcdd46b 100644 --- a/modules/mesh-delivery-forge-bridge/go.mod +++ b/modules/mesh-delivery-forge-bridge/go.mod @@ -3,7 +3,7 @@ module mesh-delivery-forge-bridge go 1.26.0 require ( - git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026210326-979644420117 + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261027011211-82f10c3bc9fa github.com/nats-io/nats-server/v2 v2.11.17 github.com/nats-io/nats.go v1.54.0 ) diff --git a/modules/mesh-delivery-forge-bridge/go.sum b/modules/mesh-delivery-forge-bridge/go.sum index e8660dd..2a278e0 100644 --- a/modules/mesh-delivery-forge-bridge/go.sum +++ b/modules/mesh-delivery-forge-bridge/go.sum @@ -1,5 +1,9 @@ git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026210326-979644420117 h1:88CNZOoQgu7YPYKW16qkWT8Gw9Ucg07aTrIImA2nYUk= git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026210326-979644420117/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261027010747-7e99c354ac7f h1:aWqn4HHTPUp/+28h7b8uFqQptAYvQFs05CALMHN7SkI= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261027010747-7e99c354ac7f/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261027011211-82f10c3bc9fa h1:tT6sl3vY0iVAXw+mqy3iGKUBUjVBRWvQvKUOTmTyWwk= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261027011211-82f10c3bc9fa/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op h1:Z/MZK75wC/NSrkgqeNIa7jexam9uWzhLmFTSCPI/kn0= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op/go.mod h1:FQyySiasQQM8735Ddel3MRojmy4dA1IqCeyJ5jmPMbI= github.com/google/go-tpm v0.9.8 h1:slArAR9Ft+1ybZu0lBwpSmpwhRXaa85hWtMinMyRAWo= diff --git a/modules/mesh-delivery-forge-bridge/internal/bridge/bridge.go b/modules/mesh-delivery-forge-bridge/internal/bridge/bridge.go index e65f8fd..45f4334 100644 --- a/modules/mesh-delivery-forge-bridge/internal/bridge/bridge.go +++ b/modules/mesh-delivery-forge-bridge/internal/bridge/bridge.go @@ -78,6 +78,24 @@ type pull struct { ModuleDirsSaid bool `json:"module_dirs_said"` Base string `json:"base"` MergeSHA string `json:"merge_commit_sha"` + // Paths are the files the pull request changes (a removed or renamed one included), PathsTruncated says the forge + // cut the list; nil when the forge did not say them. + Paths *[]string `json:"paths"` + PathsTruncated bool `json:"paths_truncated"` +} + +// touchesPlacement says whether a pull request may change a placement file: a changed path under +// delivery.PlacementDir, or paths the forge did not say or cut (the safe direction: the files are read). +func (p pull) touchesPlacement() bool { + if p.Paths == nil || p.PathsTruncated { + return true + } + for _, path := range *p.Paths { + if strings.HasPrefix(path, delivery.PlacementDir+"/") { + return true + } + } + return false } func unreadable(subject, why string, args ...any) error { @@ -98,7 +116,9 @@ type Listed func(owner, repo, sha, dir string) (map[string]string, error) // ConvertWith is Convert; with list, a merge of the catalogue repository (delivery.CatalogueRepository) also carries every // placement file of its merge commit (novox/hq ADR 0549 §2, issue 1126), read whole or not at all: a forge that cannot -// say is ErrRetry, as for the manifests, so the indexer never takes a part of placement for all of it. +// say is ErrRetry, as for the manifests, so the indexer never takes a part of placement for all of it. So does an +// updated pull request of the catalogue that touches placement/ (touchesPlacement), with every placement file at its +// head, which the indexer writes as placement candidates (ADR 0549 §4: the nodes it places answer it before it merges). func ConvertWith(subject string, data []byte, trees Trees, files Files, list Listed) (Message, error) { var action string switch subject { @@ -138,6 +158,13 @@ func ConvertWith(subject string, data []byte, trees Trees, files Files, list Lis return Message{}, fmt.Errorf("%w: %s: the manifests of %s/%s at the head %s: %v", ErrRetry, subject, p.Owner, p.Repo, p.HeadSHA, err) } } + if action == delivery.PullUpdated && list != nil && delivery.SameRepository(f.Repository, delivery.CatalogueRepository) && p.touchesPlacement() { + var err error + if f.Placements, err = placementsOf(p.Owner, p.Repo, p.HeadSHA, list, files); err != nil { + return Message{}, fmt.Errorf("%w: %s: the placement files of %s at the head %s: %v", ErrRetry, subject, f.Repository, p.HeadSHA, err) + } + f.PlacementsRead = true + } event, err := delivery.ForgeEvent(f.Repository, delivery.ForgePull) if err != nil { return Message{}, unreadable(subject, "%v", err) diff --git a/modules/mesh-delivery-forge-bridge/internal/bridge/files_test.go b/modules/mesh-delivery-forge-bridge/internal/bridge/files_test.go index d4afe54..3d07f17 100644 --- a/modules/mesh-delivery-forge-bridge/internal/bridge/files_test.go +++ b/modules/mesh-delivery-forge-bridge/internal/bridge/files_test.go @@ -146,6 +146,55 @@ func TestPullWithKnownDirsCarriesTheSameAtTheHead(t *testing.T) { // 1448/AC2 if f.Manifests != nil || f.Locks != nil || len(forge.calls) != 0 { t.Errorf("manifests %v locks %v calls %v", f.Manifests, f.Locks, forge.calls) } + + // Issue 1126 (ADR 0549 §4): an updated pull request of the catalogue that touches placement/ carries every placement + // file at its head, read whole or not at all; one that touches none, or of another repository, carries none and lists + // nothing; paths the forge did not say or cut are read as touching; a closed one carries none. + forge = &forgeFiles{have: map[string][]byte{"modules/a/module.json": manifest, + "placement/ace.json": []byte(`{"v":1,"node":"ace","modules":[]}`), "placement/g14.json": []byte(`{"v":1,"node":"g14","modules":[]}`)}} + listing := map[string]string{"ace.json": "b1", "g14.json": "b2", "README.md": "b3"} + var listedAt []string + list := func(owner, repo, sha, dir string) (map[string]string, error) { + listedAt = append(listedAt, owner+"/"+repo+"@"+sha+":"+dir) + return listing, nil + } + all := map[string]string{"ace": `{"v":1,"node":"ace","modules":[]}`, "g14": `{"v":1,"node":"g14","modules":[]}`} + catalogue := map[string]any{"repo": "mesh-catalog", "paths": []string{"placement/g14.json"}} + for _, row := range []struct { + name string + subject string + body map[string]any + want map[string]string + read bool + }{ + {"a catalogue pull touching placement", updatedSubject, catalogue, all, true}, + {"a catalogue pull touching no placement", updatedSubject, map[string]any{"repo": "mesh-catalog"}, nil, false}, + {"a catalogue pull whose paths are cut", updatedSubject, map[string]any{"repo": "mesh-catalog", "paths_truncated": true}, all, true}, + {"a catalogue pull whose paths are not said", updatedSubject, map[string]any{"repo": "mesh-catalog", "paths": nil}, all, true}, + {"another repository's pull touching placement/", updatedSubject, map[string]any{"paths": []string{"placement/g14.json"}}, nil, false}, + {"a closed catalogue pull", closedSubject, catalogue, nil, false}, + } { + listedAt = nil + body := updated(row.body) + msg, err := ConvertWith(row.subject, body, nil, forge.get, list) + if err != nil { + t.Fatalf("%s: %v", row.name, err) + } + f, err := delivery.DecodeForge(msg.Data) + if err != nil { + t.Fatalf("%s: %v", row.name, err) + } + if f.PlacementsRead != row.read || !reflect.DeepEqual(f.Placements, row.want) || (len(listedAt) > 0) != row.read { + t.Errorf("%s: placements %v (read %v), listed %v", row.name, f.Placements, f.PlacementsRead, listedAt) + } + if row.read && listedAt[0] != "novox/mesh-catalog@"+head+":placement" { + t.Errorf("%s: listed at %v, want the head", row.name, listedAt) + } + } + listing = map[string]string{"gone.json": "b5"} + if _, err := ConvertWith(updatedSubject, updated(catalogue), nil, forge.get, list); !errors.Is(err, ErrRetry) || !strings.Contains(err.Error(), "placement/gone.json") { + t.Errorf("a placement file the forge cannot give at the head: %v", err) + } } func TestAForgeErrorOrAMissingManifestIsRetryAndALockNotFoundIsLeftOut(t *testing.T) { // 1448/AC3