fix: deduplicate cookbook manifest files by path

cookbooks.go

@@ -101,20 +101,35 @@ type Cookbook struct {
 }
 
 // AllFiles flattens the flat all_files manifest and all nine per-segment slices
-// into a single slice. A server response populates one shape or the other, so
-// the merge yields the cookbook's files in either case.
+// into a single slice, deduplicated by path.
+//
+// Most servers populate one shape or the other, but nothing guarantees it, and
+// a file listed in both must still yield one entry: callers turn each entry
+// into work (Download turns it into a file to fetch and write), so a duplicate
+// means two writers for one path. The all_files entry wins, and within the
+// per-segment slices the first occurrence does.
 func (cb *Cookbook) AllFiles() []CookbookFileRef {
-	all := make([]CookbookFileRef, 0,
-		len(cb.AllFilesManifest)+
-			len(cb.Files)+len(cb.Definitions)+len(cb.Libraries)+
-			len(cb.Attributes)+len(cb.Recipes)+len(cb.Providers)+
-			len(cb.Resources)+len(cb.RootFiles)+len(cb.Templates))
-	all = append(all, cb.AllFilesManifest...)
+	total := len(cb.AllFilesManifest) +
+		len(cb.Files) + len(cb.Definitions) + len(cb.Libraries) +
+		len(cb.Attributes) + len(cb.Recipes) + len(cb.Providers) +
+		len(cb.Resources) + len(cb.RootFiles) + len(cb.Templates)
+	all := make([]CookbookFileRef, 0, total)
+	seen := make(map[string]bool, total)
+	add := func(refs []CookbookFileRef) {
+		for _, ref := range refs {
+			if seen[ref.Path] {
+				continue
+			}
+			seen[ref.Path] = true
+			all = append(all, ref)
+		}
+	}
+	add(cb.AllFilesManifest)
 	for _, seg := range [][]CookbookFileRef{
 		cb.Files, cb.Definitions, cb.Libraries, cb.Attributes,
 		cb.Recipes, cb.Providers, cb.Resources, cb.RootFiles, cb.Templates,
 	} {
-		all = append(all, seg...)
+		add(seg)
 	}
 	return all
 }

cookbooks_test.go

@@ -7,11 +7,13 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/http/httptest"
 	"os"
 	"path/filepath"
 	"slices"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"testing"
 
 	"github.com/cinc-project/cinc-api/internal/cinctest"
@@ -691,3 +693,55 @@ func TestLocalCookbookFromDir_SkipsDanglingSymlink(t *testing.T) {
 		t.Errorf("packed %+v, want just metadata.rb", cb.files)
 	}
 }
+
+// A server may populate both the flat all_files manifest and the per-segment
+// slices. Merging them must not yield the same file twice: Download turns each
+// entry into a job, and two jobs for one path race to write the same file.
+func TestAllFiles_DedupesByPath(t *testing.T) {
+	cb := &Cookbook{
+		AllFilesManifest: []CookbookFileRef{
+			{Path: "recipes/default.rb", Checksum: "aa"},
+			{Path: "metadata.rb", Checksum: "bb"},
+		},
+		Recipes:   []CookbookFileRef{{Path: "recipes/default.rb", Checksum: "aa"}},
+		RootFiles: []CookbookFileRef{{Path: "metadata.rb", Checksum: "bb"}},
+	}
+	all := cb.AllFiles()
+	if len(all) != 2 {
+		t.Fatalf("AllFiles() returned %d refs, want 2: %+v", len(all), all)
+	}
+	seen := map[string]bool{}
+	for _, ref := range all {
+		if seen[ref.Path] {
+			t.Errorf("duplicate path %q", ref.Path)
+		}
+		seen[ref.Path] = true
+	}
+}
+
+// Download must fetch and write each cookbook file exactly once, even when the
+// manifest lists it in both shapes.
+func TestDownload_FetchesEachFileOnce(t *testing.T) {
+	var fetches int64
+	files := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		atomic.AddInt64(&fetches, 1)
+		w.Write([]byte("# default recipe\n"))
+	}))
+	defer files.Close()
+
+	srv := cinctest.New(t)
+	srv.Handle("GET /organizations/o/cookbooks/nginx/1.0.0", cinctest.Route{
+		Body: fmt.Sprintf(`{"cookbook_name":"nginx","version":"1.0.0",
+			"all_files":[{"path":"recipes/default.rb","url":%q}],
+			"recipes":[{"path":"recipes/default.rb","url":%q}]}`,
+			files.URL+"/f", files.URL+"/f"),
+	})
+	c := newTestClient(t, srv.Server)
+	dest := t.TempDir()
+	if err := c.Cookbooks.Download(context.Background(), "nginx", "1.0.0", dest); err != nil {
+		t.Fatalf("Download: %v", err)
+	}
+	if got := atomic.LoadInt64(&fetches); got != 1 {
+		t.Errorf("bookshelf fetches = %d, want 1", got)
+	}
+}