From 78d57e1cc6c839f83e40dc06961126dd78504bd6 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 5 Nov 2026 19:49:27 +0100 Subject: [PATCH] A transient forge failure is asked again; only a directory missing from the tree is cached per head (issue 1921) Claude-Session: https://claude.ai/code/session_01NcbgfFSnn24M3GcqttoU7i --- .../cmd/mesh-delivery-indexer/named.go | 2 +- .../cmd/mesh-delivery-indexer/named_test.go | 10 ++++ .../internal/index/candidates.go | 3 +- .../internal/index/packages.go | 25 +++++++-- .../internal/index/previews_test.go | 53 +++++++++++-------- 5 files changed, 66 insertions(+), 27 deletions(-) diff --git a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named.go b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named.go index 791ca492..56b8ff07 100644 --- a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named.go +++ b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named.go @@ -102,7 +102,7 @@ func filesVia(call toolCaller) func(ctx context.Context, repository, sha string, } for _, d := range dirs { if _, ok := a.Files[d]; !ok { - return nil, fmt.Errorf("the answer of %s lists no files of %q", commitTreesTool, d) + return nil, fmt.Errorf("the answer of %s lists no files of %q: %w", commitTreesTool, d, index.ErrNotInTree) } } return a.Files, nil diff --git a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named_test.go b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named_test.go index ed7c21af..e8153b5c 100644 --- a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named_test.go +++ b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "mesh-delivery-indexer/internal/index" "slices" "strings" "testing" @@ -50,6 +51,15 @@ func TestTheTrunkIsTheDirectoriesTheForgeListsTrees(t *testing.T) { // 1900/AC3 if _, has := gotArgs.(map[string]any)["paths"]; has { t.Error("the trunk asked for paths") } + // A directory the answer lists no files of is a failure of the tree (index.ErrNotInTree); a call that fails is not. + noDir := func(string, any) (json.RawMessage, error) { return json.RawMessage(`{"files":{}}`), nil } + if _, err := filesVia(noDir)(context.Background(), "novox/mesh-sdk", strings.Repeat("a", 40), []string{"go"}); !errors.Is(err, index.ErrNotInTree) { + t.Errorf("a missing directory: %v, want ErrNotInTree", err) + } + down := func(string, any) (json.RawMessage, error) { return nil, errors.New("down") } + if _, err := filesVia(down)(context.Background(), "novox/mesh-sdk", strings.Repeat("a", 40), []string{"go"}); err == nil || errors.Is(err, index.ErrNotInTree) { + t.Errorf("a failed call: %v, want an error that is not ErrNotInTree", err) + } // The build the indexer runs is the engine's word (Runs.FP of its composition verb), asked of its own machine. var asked string own := func(key string, body any) (json.RawMessage, error) { diff --git a/modules/mesh-delivery-indexer/internal/index/candidates.go b/modules/mesh-delivery-indexer/internal/index/candidates.go index 46f7d37d..3e55a248 100644 --- a/modules/mesh-delivery-indexer/internal/index/candidates.go +++ b/modules/mesh-delivery-indexer/internal/index/candidates.go @@ -121,7 +121,8 @@ type Indexer struct { filling atomic.Bool // a FillTaken runs in the background (KickFill) fillAgain atomic.Bool // a kick arrived while it ran atHeads map[string]map[string]delivery.PublishedPackages // a member head's published Go modules, by repository@head (candidatePublished) - failedAt map[string]string // the head of a repository whose packages the forge could not list, by repository: said once, not asked again until the head changes + saidAt map[string]string // the head of a repository whose failed read was said, by repository: said once per head + failedAt map[string]string // the head of a repository whose tree lacks a directory asked, by repository: not asked again until the head changes // OwnBuild answers the fingerprint of the build this program runs, from the node-engine's seat; nil knows none. OwnBuild func(ctx context.Context) (string, error) underSeats bool // a dry run inside judge.WithSeats: its judge passes do not hold the seat list (verdicts.go) diff --git a/modules/mesh-delivery-indexer/internal/index/packages.go b/modules/mesh-delivery-indexer/internal/index/packages.go index b7cb7a6d..bda1e48e 100644 --- a/modules/mesh-delivery-indexer/internal/index/packages.go +++ b/modules/mesh-delivery-indexer/internal/index/packages.go @@ -2,6 +2,7 @@ package index import ( "context" + "errors" "fmt" "maps" "path" @@ -313,17 +314,29 @@ func (ix *Indexer) candidatePublished(ctx context.Context, members map[string]st } var err error if got, err = ix.packagesAt(ctx, repo, head, ms, imports[repo]); err != nil { - ix.logf("%s at the head %s: its packages are not read, so the candidate takes their published version (said once for this head): %v", repo, head, err) + // A failure of the tree itself (the directory is not in the tree at that head) no retry changes: it is asked once per + // head. Any other failure may be transient: it is asked again at the next pass, and said once per head. ix.mu.Lock() - if ix.failedAt == nil { - ix.failedAt = map[string]string{} + said := ix.saidAt[repo] == head + if ix.saidAt == nil { + ix.saidAt = map[string]string{} + } + ix.saidAt[repo] = head + if errors.Is(err, ErrNotInTree) { + if ix.failedAt == nil { + ix.failedAt = map[string]string{} + } + ix.failedAt[repo] = head } - ix.failedAt[repo] = head ix.mu.Unlock() + if !said { + ix.logf("%s at the head %s: its packages are not read, so the candidate takes their published version: %v", repo, head, err) + } continue } ix.mu.Lock() delete(ix.failedAt, repo) + delete(ix.saidAt, repo) if ix.atHeads == nil || len(ix.atHeads) >= maxHeadsKept { ix.atHeads = map[string]map[string]delivery.PublishedPackages{} } @@ -335,5 +348,9 @@ func (ix *Indexer) candidatePublished(ctx context.Context, members map[string]st return out } +// ErrNotInTree is a failure of the tree itself: a directory asked of the forge is not in the tree at that commit. No retry +// changes it, so a head that fails so is asked once (candidatePublished). +var ErrNotInTree = errors.New("the directory is not in the tree at that commit") + // maxHeadsKept bounds the member heads' packages kept between recomputes; past it they are read again. const maxHeadsKept = 64 diff --git a/modules/mesh-delivery-indexer/internal/index/previews_test.go b/modules/mesh-delivery-indexer/internal/index/previews_test.go index 63060d55..1d95b041 100644 --- a/modules/mesh-delivery-indexer/internal/index/previews_test.go +++ b/modules/mesh-delivery-indexer/internal/index/previews_test.go @@ -405,28 +405,39 @@ func TestAPreviewHeardBeforeItsPullRequestGivesTheFingerprintWhenTheCandidateIsW } }) } - // Issue 1921: a head whose packages the forge cannot list is said once and asked once, however many passes read it; the - // entry is dropped when the repository's head changes, and the new head is asked. - t.Run("a failing head is said and asked once, until the head changes", func(t *testing.T) { - ix, _, _ := previewBed(t, false) - var logs strings.Builder - ix.Logf = func(format string, args ...any) { logs.WriteString(fmt.Sprintf(format, args...) + "\n") } - asked := 0 - ix.Files = func(context.Context, string, string, []string) (map[string]map[string]string, error) { - asked++ - return nil, errors.New("the forge does not answer") - } - held := map[string]delivery.Module{"x": {Inputs2: &delivery.KeptInputs2{Packages: []string{"git.novox.be/novox/mesh-sdk/go"}}}} - for _, head := range []string{h1, h1, h2} { - ix.candidatePublished(context.Background(), map[string]string{"novox/mesh-sdk": head}, held) - if head == h1 && (asked != 1 || strings.Count(logs.String(), "its packages are not read") != 1) { - t.Fatalf("after head %s: asked %d, journal %q: want one ask and one line", head, asked, logs.String()) + // Issue 1921: a head whose tree lacks the directory asked is asked once, however many passes read it, until the head changes; + // any other failure (the forge does not answer) is asked again at every pass and said once per head. + for _, row := range []struct { + name string + err error + askedAfterTwo int // the same head read twice + askedAfterNew int // then another head + linesAfterNew int + }{ + {"the directory is not in the tree", fmt.Errorf("listing: %w", ErrNotInTree), 1, 2, 2}, + {"the forge does not answer", errors.New("the forge does not answer"), 2, 3, 2}, + } { + t.Run("a failing head: "+row.name, func(t *testing.T) { + ix, _, _ := previewBed(t, false) + var logs strings.Builder + ix.Logf = func(format string, args ...any) { logs.WriteString(fmt.Sprintf(format, args...) + "\n") } + asked := 0 + ix.Files = func(context.Context, string, string, []string) (map[string]map[string]string, error) { + asked++ + return nil, row.err } - } - if asked != 2 || strings.Count(logs.String(), "its packages are not read") != 2 { - t.Errorf("after a new head: asked %d, journal %q: want it asked and said again", asked, logs.String()) - } - }) + held := map[string]delivery.Module{"x": {Inputs2: &delivery.KeptInputs2{Packages: []string{"git.novox.be/novox/mesh-sdk/go"}}}} + for pass, head := range []string{h1, h1, h2} { + ix.candidatePublished(context.Background(), map[string]string{"novox/mesh-sdk": head}, held) + if pass == 1 && (asked != row.askedAfterTwo || strings.Count(logs.String(), "its packages are not read") != 1) { + t.Fatalf("after two passes over %s: asked %d (want %d), journal %q: want one line", head, asked, row.askedAfterTwo, logs.String()) + } + } + if asked != row.askedAfterNew || strings.Count(logs.String(), "its packages are not read") != row.linesAfterNew { + t.Errorf("asked %d (want %d), journal %q (want %d lines)", asked, row.askedAfterNew, logs.String(), row.linesAfterNew) + } + }) + } } func TestAPreviewNamingNoManifestOfTheModulesDirectoryGivesAReason(t *testing.T) {