From 1e31a30d7313539f3fd475e42666d6c42a0b9998 Mon Sep 17 00:00:00 2001 From: Pankaj Singh Date: Mon, 28 Sep 2026 16:31:05 +0530 Subject: [PATCH 01/13] feat: match a customer-configured manifest filename ASTRO_COSMOS_BOOST_MANIFEST_NAME, when set, replaces manifest.json as the only filename Cosmos Boost pre-deploy discovery matches. A customer whose manifest isn't named manifest.json (and may keep other, differently-purposed manifests in the same directory) had nothing discovered at all; setting this env var to their manifest's actual name picks up exactly that file, wherever it sits in the tree, and leaves every other file untouched. Unset, behavior is unchanged. Co-Authored-By: Claude --- pkg/cosmosboost/precompute/discover.go | 10 ++- pkg/cosmosboost/precompute/precompute.go | 20 ++++-- pkg/cosmosboost/precompute/precompute_test.go | 62 +++++++++++++++++++ pkg/cosmosboost/predeploy.go | 15 ++++- pkg/cosmosboost/predeploy_test.go | 16 +++++ 5 files changed, 113 insertions(+), 10 deletions(-) diff --git a/pkg/cosmosboost/precompute/discover.go b/pkg/cosmosboost/precompute/discover.go index d5190817b..6ee2dcbd8 100644 --- a/pkg/cosmosboost/precompute/discover.go +++ b/pkg/cosmosboost/precompute/discover.go @@ -52,14 +52,18 @@ var manifestSkipDirs = map[string]bool{ gitDir: true, // VCS internals can't hold a project's manifest } -// findManifests walks root and returns manifest.json file paths. +// findManifests walks root and returns file paths named manifestFile, or +// name (when set) instead - for a manifest written under a different name. // // A manifest whose parent directory is itself a discovered project root is // omitted: that project's folder hash already covers a manifest sitting in its // root. Manifests elsewhere — most importantly a standalone one shipped for a // manifest-only (DBT_MANIFEST) deployment, or a project's target/manifest.json — // each get their own sidecar. -func findManifests(root string, projectDirs map[string]bool) ([]string, error) { +func findManifests(root string, projectDirs map[string]bool, name string) ([]string, error) { + if name == "" { + name = manifestFile + } var manifests []string err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { @@ -72,7 +76,7 @@ func findManifests(root string, projectDirs map[string]bool) ([]string, error) { } return nil } - if d.Name() != manifestFile { + if d.Name() != name { return nil } if projectDirs[filepath.Dir(path)] { diff --git a/pkg/cosmosboost/precompute/precompute.go b/pkg/cosmosboost/precompute/precompute.go index 8b6866885..77ae14cf6 100644 --- a/pkg/cosmosboost/precompute/precompute.go +++ b/pkg/cosmosboost/precompute/precompute.go @@ -45,6 +45,11 @@ type Options struct { // SlimManifest also writes a slim, field-filtered copy of each discovered // manifest.json (see buildSlimManifest) next to its sidecar. SlimManifest bool + + // ManifestName, when set, replaces manifest.json as the filename + // findManifests matches - for a manifest under a different name, or to + // pick one out of several valid manifests in the same directory. + ManifestName string } // Run finds every dbt project (a directory with dbt_project.yml) and standalone @@ -79,7 +84,7 @@ func Run(roots []string, version string, opts Options) (Summary, error) { manifests := map[string]bool{} for _, root := range roots { - found, err := findManifests(root, projectDirs) + found, err := findManifests(root, projectDirs, opts.ManifestName) if err != nil { return Summary{}, fmt.Errorf("scanning %q for manifests: %w", root, err) } @@ -140,13 +145,16 @@ func processProject(dir, version string, opts Options) Result { " in dbt_project.yml hold unresolved Jinja templates; using the dbt default directories for exclusion (the real ones may add cache churn)" } - // A manifest.json in the project root is not a unit of its own - its .astro/ - // is this project's, so it would collide with the sidecar written below - and - // findManifests skips it for that reason. Slim it here instead, leaving the - // project's own hash as the anchor the pointer hangs off. + // A manifest.json (or opts.ManifestName) in the project root is not a unit + // of its own - its .astro/ is this project's - so findManifests skips it. + // Slim it here instead, leaving the project's own hash as the anchor. var filtered *FilteredManifest if opts.SlimManifest { - if doc, _, isDbt, readErr := readManifestDoc(filepath.Join(dir, manifestFile)); readErr == nil && isDbt { + name := manifestFile + if opts.ManifestName != "" { + name = opts.ManifestName + } + if doc, _, isDbt, readErr := readManifestDoc(filepath.Join(dir, name)); readErr == nil && isDbt { // Nothing mutates doc afterward here, unlike processManifest. data, _ := json.Marshal(buildSlimManifest(doc, version)) if filtered, r.Err = writeSlimManifest(dir, data); r.Err != nil { diff --git a/pkg/cosmosboost/precompute/precompute_test.go b/pkg/cosmosboost/precompute/precompute_test.go index 1177db052..e4a2e5275 100644 --- a/pkg/cosmosboost/precompute/precompute_test.go +++ b/pkg/cosmosboost/precompute/precompute_test.go @@ -126,6 +126,68 @@ func TestRunSkipsNonDBTManifest(t *testing.T) { } } +// TestRunHonorsManifestNameOverride: two valid dbt manifests share a +// directory, neither named manifest.json. ManifestName picks up exactly the +// one it names, leaving the other alone. +func TestRunHonorsManifestNameOverride(t *testing.T) { + root := t.TempDir() + writeFiles(t, root, map[string]string{ + "manifests_per_schedule/manifest_global_daily_schedule.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{"model.daily":{"name":"daily"}}}`, + "manifests_per_schedule/manifest_full.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{"model.full":{"name":"full"}}}`, + }) + want := filepath.Join(root, "manifests_per_schedule", "manifest_full.json") + + summary, err := Run([]string{root}, "test", Options{ManifestName: "manifest_full.json"}) + if err != nil { + t.Fatal(err) + } + if len(summary.Results) != 1 || summary.Results[0].Path != want || summary.Results[0].Err != nil { + t.Fatalf("want 1 result for manifest_full.json only, got %+v", summary.Results) + } + mustExist(t, filepath.Join(root, "manifests_per_schedule", sidecarDir, sidecarName)) +} + +// TestRunManifestNameOverrideExcludesDefaultName: ManifestName replaces +// manifest.json rather than adding to it - a manifest.json sitting alongside +// the named file is left untouched. +func TestRunManifestNameOverrideExcludesDefaultName(t *testing.T) { + root := t.TempDir() + writeFiles(t, root, map[string]string{ + "shipped/manifest.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, + "shipped/manifest_full.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, + }) + want := filepath.Join(root, "shipped", "manifest_full.json") + + summary, err := Run([]string{root}, "test", Options{ManifestName: "manifest_full.json"}) + if err != nil { + t.Fatal(err) + } + if len(summary.Results) != 1 || summary.Results[0].Path != want { + t.Fatalf("want 1 result for manifest_full.json only, manifest.json must be ignored: %+v", summary.Results) + } +} + +// TestRunManifestNameOverrideInProjectRootIsSlimmedNotDiscovered: a +// ManifestName file in a project root is slimmed by processProject, not +// discovered as its own unit (same as manifest.json there). +func TestRunManifestNameOverrideInProjectRootIsSlimmedNotDiscovered(t *testing.T) { + root := t.TempDir() + writeFiles(t, root, map[string]string{ + "proj/dbt_project.yml": "name: shop\n", + "proj/models/a.sql": "select 1", + "proj/manifest_full.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json","project_name":"shop"},"nodes":{"model.shop.orders":{"original_file_path":"models/orders.sql","package_name":"shop","resource_type":"model","fqn":["shop","orders"]}}}`, + }) + + summary, err := Run([]string{root}, "test", Options{SlimManifest: true, ManifestName: "manifest_full.json"}) + if err != nil { + t.Fatal(err) + } + if len(summary.Results) != 1 || summary.Results[0].Kind != kindProject || summary.Results[0].Err != nil { + t.Fatalf("want 1 project-only result, got %+v", summary.Results) + } + mustExist(t, filepath.Join(root, "proj", sidecarDir, slimManifestName)) +} + // TestRunWarnsOnTemplatedPackagesPath verifies a project whose packages-install-path // is a Jinja template still gets stamped, but the Result carries a non-fatal warning. func TestRunWarnsOnTemplatedPackagesPath(t *testing.T) { diff --git a/pkg/cosmosboost/predeploy.go b/pkg/cosmosboost/predeploy.go index dff788924..720665ffc 100644 --- a/pkg/cosmosboost/predeploy.go +++ b/pkg/cosmosboost/predeploy.go @@ -28,6 +28,16 @@ func slimManifestEnabled() bool { return value == "" || util.CheckEnvBool(value) } +// manifestNameEnvVar names the filename discovery matches in place of +// manifest.json (see precompute.Options.ManifestName). +const manifestNameEnvVar = "ASTRO_COSMOS_BOOST_MANIFEST_NAME" + +// manifestNameOverride returns "" when unset, leaving manifest.json as the +// only name discovery matches. +func manifestNameOverride() string { + return strings.TrimSpace(os.Getenv(manifestNameEnvVar)) +} + // PreDeploy runs the Cosmos Boost pre-deploy step over path: every dbt project // (dbt_project.yml) gets a .astro/dbt_metadata.json sidecar carrying its // content hash, which the Cosmos Boost plugin uses as a cache version key at @@ -35,7 +45,10 @@ func slimManifestEnabled() bool { // manifest.json gets a hash sidecar too, plus a slim, field-filtered copy for // the plugin to load in place of the full manifest at DAG-parse time. func PreDeploy(path string) error { - opts := precompute.Options{SlimManifest: slimManifestEnabled()} + opts := precompute.Options{ + SlimManifest: slimManifestEnabled(), + ManifestName: manifestNameOverride(), + } summary, err := precompute.Run([]string{path}, version.CurrVersion, opts) if err != nil { return fmt.Errorf("running the Cosmos Boost pre-deploy step: %w", err) diff --git a/pkg/cosmosboost/predeploy_test.go b/pkg/cosmosboost/predeploy_test.go index 7398abf14..6dc0b0c26 100644 --- a/pkg/cosmosboost/predeploy_test.go +++ b/pkg/cosmosboost/predeploy_test.go @@ -101,6 +101,22 @@ func TestSlimManifestEnabled(t *testing.T) { } } +// TestPreDeployRespectsManifestNameEnvVar: a manifest.json-only discovery +// misses a differently-named file, but stamps it once the env var names it. +func TestPreDeployRespectsManifestNameEnvVar(t *testing.T) { + dir := t.TempDir() + manifest := `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}` + require.NoError(t, os.WriteFile(filepath.Join(dir, "manifest_full.json"), []byte(manifest), 0o644)) + + require.NoError(t, PreDeploy(dir)) + _, err := os.Stat(filepath.Join(dir, ".astro")) + require.True(t, os.IsNotExist(err), "a non-manifest.json name is invisible to discovery without the override") + + t.Setenv(manifestNameEnvVar, "manifest_full.json") + require.NoError(t, PreDeploy(dir)) + require.FileExists(t, filepath.Join(dir, artifactRelPath)) +} + func TestPreDeployNoDbtContentIsANoOp(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "app.py"), []byte("print('hi')"), 0o644)) From c789962d7a2319eb1ddafcb028c7ce99de5624d3 Mon Sep 17 00:00:00 2001 From: Pankaj Singh Date: Tue, 29 Sep 2026 23:26:34 +0530 Subject: [PATCH 02/13] fix: scope manifest name override per directory ASTRO_COSMOS_BOOST_MANIFEST_NAME now holds a JSON object mapping a directory (relative to the deployed path, "." for its root) to the filename discovery should match there, instead of one name applied to the whole deploy. A single global name couldn't represent different projects using different manifest naming conventions in one deploy; an unlisted directory still falls back to manifest.json. Also rejects malformed JSON and any value that isn't a bare filename (a path there would silently match nothing in findManifests' basename comparison), failing the deploy step instead of stamping nothing silently. Co-Authored-By: Claude --- pkg/cosmosboost/precompute/discover.go | 29 +++++++++--- pkg/cosmosboost/precompute/precompute.go | 38 ++++++++------- pkg/cosmosboost/precompute/precompute_test.go | 46 +++++++++++++++---- pkg/cosmosboost/predeploy.go | 39 ++++++++++++---- pkg/cosmosboost/predeploy_test.go | 22 ++++++++- 5 files changed, 130 insertions(+), 44 deletions(-) diff --git a/pkg/cosmosboost/precompute/discover.go b/pkg/cosmosboost/precompute/discover.go index 6ee2dcbd8..4767578b2 100644 --- a/pkg/cosmosboost/precompute/discover.go +++ b/pkg/cosmosboost/precompute/discover.go @@ -52,18 +52,33 @@ var manifestSkipDirs = map[string]bool{ gitDir: true, // VCS internals can't hold a project's manifest } -// findManifests walks root and returns file paths named manifestFile, or -// name (when set) instead - for a manifest written under a different name. +// effectiveManifestName is the filename expected in dir: overrides[rel], +// where rel is dir's slash-separated path relative to root ("." for root +// itself), or manifestFile when overrides is empty or has no entry for rel. +func effectiveManifestName(overrides map[string]string, root, dir string) string { + if len(overrides) == 0 { + return manifestFile + } + rel, err := filepath.Rel(root, dir) + if err != nil { + return manifestFile + } + if name, ok := overrides[filepath.ToSlash(rel)]; ok && name != "" { + return name + } + return manifestFile +} + +// findManifests walks root and returns file paths matching, in each +// directory, the name effectiveManifestName resolves for it - manifestFile +// by default, or a per-directory override. // // A manifest whose parent directory is itself a discovered project root is // omitted: that project's folder hash already covers a manifest sitting in its // root. Manifests elsewhere — most importantly a standalone one shipped for a // manifest-only (DBT_MANIFEST) deployment, or a project's target/manifest.json — // each get their own sidecar. -func findManifests(root string, projectDirs map[string]bool, name string) ([]string, error) { - if name == "" { - name = manifestFile - } +func findManifests(root string, projectDirs map[string]bool, overrides map[string]string) ([]string, error) { var manifests []string err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { @@ -76,7 +91,7 @@ func findManifests(root string, projectDirs map[string]bool, name string) ([]str } return nil } - if d.Name() != name { + if d.Name() != effectiveManifestName(overrides, root, filepath.Dir(path)) { return nil } if projectDirs[filepath.Dir(path)] { diff --git a/pkg/cosmosboost/precompute/precompute.go b/pkg/cosmosboost/precompute/precompute.go index 77ae14cf6..225a03d2d 100644 --- a/pkg/cosmosboost/precompute/precompute.go +++ b/pkg/cosmosboost/precompute/precompute.go @@ -46,10 +46,13 @@ type Options struct { // manifest.json (see buildSlimManifest) next to its sidecar. SlimManifest bool - // ManifestName, when set, replaces manifest.json as the filename - // findManifests matches - for a manifest under a different name, or to - // pick one out of several valid manifests in the same directory. - ManifestName string + // ManifestNames overrides, per directory, the filename findManifests and + // processProject match instead of manifest.json - for a manifest under a + // different name, or to pick one out of several valid manifests in the + // same directory. Keyed by a directory's slash-separated path relative to + // its root ("." for the root itself); a directory with no entry still + // uses manifest.json. + ManifestNames map[string]string } // Run finds every dbt project (a directory with dbt_project.yml) and standalone @@ -72,6 +75,7 @@ func Run(roots []string, version string, opts Options) (Summary, error) { start := time.Now() projectDirs := map[string]bool{} + projectRoots := map[string]string{} // project dir -> root it was found under for _, root := range roots { found, err := findProjects(root) if err != nil { @@ -79,12 +83,13 @@ func Run(roots []string, version string, opts Options) (Summary, error) { } for _, d := range found { projectDirs[d] = true + projectRoots[d] = root } } manifests := map[string]bool{} for _, root := range roots { - found, err := findManifests(root, projectDirs, opts.ManifestName) + found, err := findManifests(root, projectDirs, opts.ManifestNames) if err != nil { return Summary{}, fmt.Errorf("scanning %q for manifests: %w", root, err) } @@ -93,13 +98,13 @@ func Run(roots []string, version string, opts Options) (Summary, error) { } } - type unit struct{ kind, path string } + type unit struct{ kind, path, root string } var units []unit for d := range projectDirs { - units = append(units, unit{kindProject, d}) + units = append(units, unit{kindProject, d, projectRoots[d]}) } for m := range manifests { - units = append(units, unit{kindManifest, m}) + units = append(units, unit{kindManifest, m, ""}) } // Composite sort key: path first, kind as the tiebreaker. NUL sorts below // every other byte, so prefix relationships between paths are preserved. @@ -117,7 +122,7 @@ func Run(roots []string, version string, opts Options) (Summary, error) { defer wg.Done() defer func() { <-sem }() // release the slot if u.kind == kindProject { - results[i] = processProject(u.path, version, opts) + results[i] = processProject(u.path, u.root, version, opts) } else { results[i] = processManifest(u.path, version, opts) } @@ -131,7 +136,8 @@ func Run(roots []string, version string, opts Options) (Summary, error) { // processProject hashes one dbt project directory and writes its sidecar. It reads // dbt_project.yml once (readDbtConfig) and threads the result through hashing and the // templated-packages warning, so the file isn't parsed more than once per project. -func processProject(dir, version string, opts Options) Result { +// root is the discovery root dir was found under, used to resolve opts.ManifestNames. +func processProject(dir, root, version string, opts Options) Result { start := time.Now() cfg := readDbtConfig(dir) hash, files, totalBytes, err := hashProject(dir, cfg) @@ -145,15 +151,13 @@ func processProject(dir, version string, opts Options) Result { " in dbt_project.yml hold unresolved Jinja templates; using the dbt default directories for exclusion (the real ones may add cache churn)" } - // A manifest.json (or opts.ManifestName) in the project root is not a unit - // of its own - its .astro/ is this project's - so findManifests skips it. - // Slim it here instead, leaving the project's own hash as the anchor. + // A manifest.json (or an opts.ManifestNames override) in the project root + // is not a unit of its own - its .astro/ is this project's - so + // findManifests skips it. Slim it here instead, leaving the project's own + // hash as the anchor. var filtered *FilteredManifest if opts.SlimManifest { - name := manifestFile - if opts.ManifestName != "" { - name = opts.ManifestName - } + name := effectiveManifestName(opts.ManifestNames, root, dir) if doc, _, isDbt, readErr := readManifestDoc(filepath.Join(dir, name)); readErr == nil && isDbt { // Nothing mutates doc afterward here, unlike processManifest. data, _ := json.Marshal(buildSlimManifest(doc, version)) diff --git a/pkg/cosmosboost/precompute/precompute_test.go b/pkg/cosmosboost/precompute/precompute_test.go index e4a2e5275..e3220dc63 100644 --- a/pkg/cosmosboost/precompute/precompute_test.go +++ b/pkg/cosmosboost/precompute/precompute_test.go @@ -127,8 +127,8 @@ func TestRunSkipsNonDBTManifest(t *testing.T) { } // TestRunHonorsManifestNameOverride: two valid dbt manifests share a -// directory, neither named manifest.json. ManifestName picks up exactly the -// one it names, leaving the other alone. +// directory, neither named manifest.json. A ManifestNames entry for that +// directory picks up exactly the one it names, leaving the other alone. func TestRunHonorsManifestNameOverride(t *testing.T) { root := t.TempDir() writeFiles(t, root, map[string]string{ @@ -137,7 +137,7 @@ func TestRunHonorsManifestNameOverride(t *testing.T) { }) want := filepath.Join(root, "manifests_per_schedule", "manifest_full.json") - summary, err := Run([]string{root}, "test", Options{ManifestName: "manifest_full.json"}) + summary, err := Run([]string{root}, "test", Options{ManifestNames: map[string]string{"manifests_per_schedule": "manifest_full.json"}}) if err != nil { t.Fatal(err) } @@ -147,9 +147,9 @@ func TestRunHonorsManifestNameOverride(t *testing.T) { mustExist(t, filepath.Join(root, "manifests_per_schedule", sidecarDir, sidecarName)) } -// TestRunManifestNameOverrideExcludesDefaultName: ManifestName replaces -// manifest.json rather than adding to it - a manifest.json sitting alongside -// the named file is left untouched. +// TestRunManifestNameOverrideExcludesDefaultName: a directory's override +// replaces manifest.json rather than adding to it - a manifest.json sitting +// alongside the named file is left untouched. func TestRunManifestNameOverrideExcludesDefaultName(t *testing.T) { root := t.TempDir() writeFiles(t, root, map[string]string{ @@ -158,7 +158,7 @@ func TestRunManifestNameOverrideExcludesDefaultName(t *testing.T) { }) want := filepath.Join(root, "shipped", "manifest_full.json") - summary, err := Run([]string{root}, "test", Options{ManifestName: "manifest_full.json"}) + summary, err := Run([]string{root}, "test", Options{ManifestNames: map[string]string{"shipped": "manifest_full.json"}}) if err != nil { t.Fatal(err) } @@ -168,8 +168,9 @@ func TestRunManifestNameOverrideExcludesDefaultName(t *testing.T) { } // TestRunManifestNameOverrideInProjectRootIsSlimmedNotDiscovered: a -// ManifestName file in a project root is slimmed by processProject, not -// discovered as its own unit (same as manifest.json there). +// ManifestNames file in a project root ("." - the project dir itself) is +// slimmed by processProject, not discovered as its own unit (same as +// manifest.json there). func TestRunManifestNameOverrideInProjectRootIsSlimmedNotDiscovered(t *testing.T) { root := t.TempDir() writeFiles(t, root, map[string]string{ @@ -178,7 +179,7 @@ func TestRunManifestNameOverrideInProjectRootIsSlimmedNotDiscovered(t *testing.T "proj/manifest_full.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json","project_name":"shop"},"nodes":{"model.shop.orders":{"original_file_path":"models/orders.sql","package_name":"shop","resource_type":"model","fqn":["shop","orders"]}}}`, }) - summary, err := Run([]string{root}, "test", Options{SlimManifest: true, ManifestName: "manifest_full.json"}) + summary, err := Run([]string{root}, "test", Options{SlimManifest: true, ManifestNames: map[string]string{"proj": "manifest_full.json"}}) if err != nil { t.Fatal(err) } @@ -188,6 +189,31 @@ func TestRunManifestNameOverrideInProjectRootIsSlimmedNotDiscovered(t *testing.T mustExist(t, filepath.Join(root, "proj", sidecarDir, slimManifestName)) } +// TestRunManifestNameOverridePerDirectory: two dbt projects with different +// manifest-root conventions in one Run - ManifestNames applies each +// directory's own override independently. +func TestRunManifestNameOverridePerDirectory(t *testing.T) { + root := t.TempDir() + writeFiles(t, root, map[string]string{ + "dbt1/dbt_project.yml": "name: one\n", + "dbt1/models/a.sql": "select 1", + "dbt1/manifest_custom.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json","project_name":"one"},"nodes":{"model.one.a":{"original_file_path":"models/a.sql","package_name":"one","resource_type":"model","fqn":["one","a"]}}}`, + "dbt2/dbt_project.yml": "name: two\n", + "dbt2/models/b.sql": "select 2", + "dbt2/manifest.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json","project_name":"two"},"nodes":{"model.two.b":{"original_file_path":"models/b.sql","package_name":"two","resource_type":"model","fqn":["two","b"]}}}`, + }) + + summary, err := Run([]string{root}, "test", Options{SlimManifest: true, ManifestNames: map[string]string{"dbt1": "manifest_custom.json"}}) + if err != nil { + t.Fatal(err) + } + if len(summary.Results) != 2 { + t.Fatalf("want 2 project results, got %+v", summary.Results) + } + mustExist(t, filepath.Join(root, "dbt1", sidecarDir, slimManifestName)) // found via the override + mustExist(t, filepath.Join(root, "dbt2", sidecarDir, slimManifestName)) // found via the manifest.json default +} + // TestRunWarnsOnTemplatedPackagesPath verifies a project whose packages-install-path // is a Jinja template still gets stamped, but the Result carries a non-fatal warning. func TestRunWarnsOnTemplatedPackagesPath(t *testing.T) { diff --git a/pkg/cosmosboost/predeploy.go b/pkg/cosmosboost/predeploy.go index 720665ffc..472a0cba6 100644 --- a/pkg/cosmosboost/predeploy.go +++ b/pkg/cosmosboost/predeploy.go @@ -2,9 +2,11 @@ package cosmosboost import ( "bytes" + "encoding/json" "fmt" "io" "os" + "path/filepath" "strings" "github.com/astronomer/astro-cli/pkg/cosmosboost/precompute" @@ -28,14 +30,31 @@ func slimManifestEnabled() bool { return value == "" || util.CheckEnvBool(value) } -// manifestNameEnvVar names the filename discovery matches in place of -// manifest.json (see precompute.Options.ManifestName). +// manifestNameEnvVar holds a JSON object mapping a directory (relative to the +// deployed path, "." for its root) to the filename discovery should match +// there instead of manifest.json (see precompute.Options.ManifestNames). A +// directory with no entry keeps matching manifest.json. const manifestNameEnvVar = "ASTRO_COSMOS_BOOST_MANIFEST_NAME" -// manifestNameOverride returns "" when unset, leaving manifest.json as the -// only name discovery matches. -func manifestNameOverride() string { - return strings.TrimSpace(os.Getenv(manifestNameEnvVar)) +// manifestNameOverrides parses manifestNameEnvVar, or returns nil when unset +// (every directory keeps matching manifest.json). Every value must be a bare +// filename (no path separators): findManifests matches on a file's basename +// alone, so a path there would silently match nothing. +func manifestNameOverrides() (map[string]string, error) { + value := strings.TrimSpace(os.Getenv(manifestNameEnvVar)) + if value == "" { + return nil, nil + } + var overrides map[string]string + if err := json.Unmarshal([]byte(value), &overrides); err != nil { + return nil, fmt.Errorf("%s must be a JSON object of directory to manifest filename: %w", manifestNameEnvVar, err) + } + for dir, name := range overrides { + if name == "" || filepath.Base(name) != name { + return nil, fmt.Errorf("%s: %q for directory %q must be a bare filename, not a path", manifestNameEnvVar, name, dir) + } + } + return overrides, nil } // PreDeploy runs the Cosmos Boost pre-deploy step over path: every dbt project @@ -45,9 +64,13 @@ func manifestNameOverride() string { // manifest.json gets a hash sidecar too, plus a slim, field-filtered copy for // the plugin to load in place of the full manifest at DAG-parse time. func PreDeploy(path string) error { + manifestNames, err := manifestNameOverrides() + if err != nil { + return err + } opts := precompute.Options{ - SlimManifest: slimManifestEnabled(), - ManifestName: manifestNameOverride(), + SlimManifest: slimManifestEnabled(), + ManifestNames: manifestNames, } summary, err := precompute.Run([]string{path}, version.CurrVersion, opts) if err != nil { diff --git a/pkg/cosmosboost/predeploy_test.go b/pkg/cosmosboost/predeploy_test.go index 6dc0b0c26..850b50652 100644 --- a/pkg/cosmosboost/predeploy_test.go +++ b/pkg/cosmosboost/predeploy_test.go @@ -102,7 +102,8 @@ func TestSlimManifestEnabled(t *testing.T) { } // TestPreDeployRespectsManifestNameEnvVar: a manifest.json-only discovery -// misses a differently-named file, but stamps it once the env var names it. +// misses a differently-named file, but stamps it once the env var names it +// for the deploy root ("."). func TestPreDeployRespectsManifestNameEnvVar(t *testing.T) { dir := t.TempDir() manifest := `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}` @@ -112,11 +113,28 @@ func TestPreDeployRespectsManifestNameEnvVar(t *testing.T) { _, err := os.Stat(filepath.Join(dir, ".astro")) require.True(t, os.IsNotExist(err), "a non-manifest.json name is invisible to discovery without the override") - t.Setenv(manifestNameEnvVar, "manifest_full.json") + t.Setenv(manifestNameEnvVar, `{".": "manifest_full.json"}`) require.NoError(t, PreDeploy(dir)) require.FileExists(t, filepath.Join(dir, artifactRelPath)) } +// TestPreDeployRejectsInvalidManifestNameJSON: malformed JSON in the env var +// fails the deploy step rather than silently matching nothing. +func TestPreDeployRejectsInvalidManifestNameJSON(t *testing.T) { + dir := t.TempDir() + t.Setenv(manifestNameEnvVar, "not-json") + require.ErrorContains(t, PreDeploy(dir), manifestNameEnvVar) +} + +// TestPreDeployRejectsPathValuedManifestName: a value containing a path +// separator would never match findManifests' basename comparison, so it's +// rejected up front instead of silently stamping nothing. +func TestPreDeployRejectsPathValuedManifestName(t *testing.T) { + dir := t.TempDir() + t.Setenv(manifestNameEnvVar, `{".": "target/manifest_full.json"}`) + require.ErrorContains(t, PreDeploy(dir), manifestNameEnvVar) +} + func TestPreDeployNoDbtContentIsANoOp(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "app.py"), []byte("print('hi')"), 0o644)) From e97b4cd4e19ba611840f12c8a67cbdbd1e3ee7db Mon Sep 17 00:00:00 2001 From: Pankaj Singh Date: Tue, 29 Sep 2026 23:33:28 +0530 Subject: [PATCH 03/13] fix: skip stamping a directory with more than one valid dbt manifest The plugin resolves both the hash sidecar and the slim manifest by directory alone, with no check of which manifest produced them. If a directory legitimately holds more than one valid dbt manifest (e.g. per-schedule manifests read by different DAGs), stamping it for one manifest would risk serving that manifest's slim copy and cache hash to a DAG that actually points at the other. hasSiblingDbtManifest checks, before writing anything, whether another *.json file in the same directory also parses as a valid dbt manifest. When it does, processManifest skips both artifacts for that unit (like a non-dbt manifest, but with a Warning naming the reason) and processProject skips only the slim attachment, since the project's own tree-hash sidecar isn't manifest-specific and stays safe to write. Co-Authored-By: Claude --- pkg/cosmosboost/precompute/hash.go | 28 +++++++++ pkg/cosmosboost/precompute/precompute.go | 61 +++++++++++++++---- pkg/cosmosboost/precompute/precompute_test.go | 59 ++++++++++++++---- 3 files changed, 123 insertions(+), 25 deletions(-) diff --git a/pkg/cosmosboost/precompute/hash.go b/pkg/cosmosboost/precompute/hash.go index b99d1a5e8..b372f9ae0 100644 --- a/pkg/cosmosboost/precompute/hash.go +++ b/pkg/cosmosboost/precompute/hash.go @@ -211,6 +211,34 @@ func readManifestDoc(path string) (doc map[string]any, bytes int64, isDbt bool, return doc, bytes, true, nil } +// hasSiblingDbtManifest reports whether dir holds a *.json file, other than +// exclude, that also parses as a valid dbt manifest. The Cosmos Boost plugin +// resolves both the hash sidecar and the slim manifest by directory alone, +// with no check of which manifest produced them - so stamping dir while it +// holds more than one valid dbt manifest risks serving one manifest's +// artifacts to a DAG that points at the other. Checking .json files only +// keeps the cost bounded to plausible candidates in that single directory, +// not a tree-wide scan. +func hasSiblingDbtManifest(dir, exclude string) (bool, error) { + entries, err := os.ReadDir(dir) + if err != nil { + return false, err + } + for _, e := range entries { + if e.IsDir() || e.Name() == exclude || filepath.Ext(e.Name()) != ".json" { + continue + } + _, _, isDbt, err := readManifestDoc(filepath.Join(dir, e.Name())) + if err != nil { + return false, err + } + if isDbt { + return true, nil + } + } + return false, nil +} + // hashDocument computes the "sha256-manifest-v2" hash of an already-parsed // dbt manifest document, after removing the volatile fields dbt regenerates on // every full parse, so the value is stable across recompiles of unchanged diff --git a/pkg/cosmosboost/precompute/precompute.go b/pkg/cosmosboost/precompute/precompute.go index 225a03d2d..bdd7870f0 100644 --- a/pkg/cosmosboost/precompute/precompute.go +++ b/pkg/cosmosboost/precompute/precompute.go @@ -27,8 +27,8 @@ type Result struct { Files int // files hashed (1 for a manifest) Bytes int64 // total bytes hashed Duration time.Duration // time spent on this unit - Skipped bool // a manifest.json that isn't a dbt manifest (no sidecar written) - Warning string // non-fatal note (sidecar still written), e.g. an unresolved template + Skipped bool // no sidecar written: not a dbt manifest, or (see Warning) an ambiguous directory + Warning string // non-fatal note, e.g. an unresolved template, or why a Skipped unit was skipped Err error // non-nil if hashing, writing the sidecar, or writing the slim manifest failed } @@ -159,12 +159,24 @@ func processProject(dir, root, version string, opts Options) Result { if opts.SlimManifest { name := effectiveManifestName(opts.ManifestNames, root, dir) if doc, _, isDbt, readErr := readManifestDoc(filepath.Join(dir, name)); readErr == nil && isDbt { - // Nothing mutates doc afterward here, unlike processManifest. - data, _ := json.Marshal(buildSlimManifest(doc, version)) - if filtered, r.Err = writeSlimManifest(dir, data); r.Err != nil { + // Skip slimming (not the whole project) when another valid dbt + // manifest shares this directory: see hasSiblingDbtManifest. + ambiguous, sibErr := hasSiblingDbtManifest(dir, name) + if sibErr != nil { + r.Err = sibErr r.Duration = time.Since(start) return r } + if ambiguous { + r.Warning = joinWarnings(r.Warning, name+" not slimmed: another valid dbt manifest shares this directory") + } else { + // Nothing mutates doc afterward here, unlike processManifest. + data, _ := json.Marshal(buildSlimManifest(doc, version)) + if filtered, r.Err = writeSlimManifest(dir, data); r.Err != nil { + r.Duration = time.Since(start) + return r + } + } } } @@ -177,19 +189,28 @@ func processProject(dir, root, version string, opts Options) Result { // plus a slim, field-filtered copy of the manifest when opts asks for one (see // buildSlimManifest). A file that isn't a dbt manifest is skipped (nothing is // written) so unrelated manifest.json files in the project aren't stamped. +// +// It is also skipped when another valid dbt manifest shares its directory +// (see hasSiblingDbtManifest): both artifacts would land in the same .astro/, +// and the plugin has no way to tell which manifest they belong to, so writing +// them risks serving this manifest's slim copy and hash to a DAG that +// actually points at the sibling. func processManifest(path, version string, opts Options) Result { start := time.Now() doc, bytes, isDbt, err := readManifestDoc(path) var hash string var slimData []byte + var ambiguous bool if err == nil && isDbt { - if opts.SlimManifest { - // Marshal before hashDocument mutates doc: the slim manifest shares - // doc's nested values, so only turning it into bytes here decouples - // the two. It holds JSON-native types only, so this cannot fail. - slimData, _ = json.Marshal(buildSlimManifest(doc, version)) + if ambiguous, err = hasSiblingDbtManifest(filepath.Dir(path), filepath.Base(path)); err == nil && !ambiguous { + if opts.SlimManifest { + // Marshal before hashDocument mutates doc: the slim manifest shares + // doc's nested values, so only turning it into bytes here decouples + // the two. It holds JSON-native types only, so this cannot fail. + slimData, _ = json.Marshal(buildSlimManifest(doc, version)) + } + hash = hashDocument(doc) } - hash = hashDocument(doc) } r := Result{Kind: kindManifest, Path: path, Hash: hash, Files: 1, Bytes: bytes, Duration: time.Since(start)} switch { @@ -197,6 +218,9 @@ func processManifest(path, version string, opts Options) Result { r.Err = err case !isDbt: r.Skipped = true + case ambiguous: + r.Skipped = true + r.Warning = "another valid dbt manifest shares this directory; skipped to avoid stamping the wrong one's cache" default: dir := filepath.Dir(path) // The sidecar goes last: it carries the filtered_manifest pointer, so it @@ -214,6 +238,15 @@ func processManifest(path, version string, opts Options) Result { return r } +// joinWarnings appends add to existing, semicolon-separated, so a later +// warning doesn't overwrite an earlier one on the same Result. +func joinWarnings(existing, add string) string { + if existing == "" { + return add + } + return existing + "; " + add +} + // writeSlimManifest writes data as dir's slim manifest and returns the sidecar // pointer describing it. data must already be marshaled, so a caller that later // mutates the source doc cannot leak into it (see processManifest). @@ -239,8 +272,8 @@ func (s Summary) CountFailed() int { return n } -// CountSkipped returns the number of units skipped (manifest.json files that aren't -// dbt manifests). +// CountSkipped returns the number of units skipped: not a dbt manifest, or an +// ambiguous directory (see hasSiblingDbtManifest). func (s Summary) CountSkipped() int { n := 0 for _, r := range s.Results { @@ -271,6 +304,8 @@ func (s Summary) WriteReport(w io.Writer) { switch { case r.Err != nil: fmt.Fprintf(w, " %s %-8s %s (%v)\n", glyphFail, r.Kind, r.Path, r.Err) + case r.Skipped && r.Warning != "": + fmt.Fprintf(w, " %s %-8s %s (%s)\n", glyphLeft, r.Kind, r.Path, r.Warning) case r.Skipped: fmt.Fprintf(w, " %s %-8s %s (not a dbt manifest)\n", glyphLeft, r.Kind, r.Path) default: diff --git a/pkg/cosmosboost/precompute/precompute_test.go b/pkg/cosmosboost/precompute/precompute_test.go index e3220dc63..c36c4badc 100644 --- a/pkg/cosmosboost/precompute/precompute_test.go +++ b/pkg/cosmosboost/precompute/precompute_test.go @@ -126,10 +126,14 @@ func TestRunSkipsNonDBTManifest(t *testing.T) { } } -// TestRunHonorsManifestNameOverride: two valid dbt manifests share a -// directory, neither named manifest.json. A ManifestNames entry for that -// directory picks up exactly the one it names, leaving the other alone. -func TestRunHonorsManifestNameOverride(t *testing.T) { +// TestRunSkipsAmbiguousDirectoryEvenWithOverride: the real hazard this PR's +// override could otherwise hit head-on - two valid dbt manifests share a +// directory (e.g. per-schedule manifests, each read by a different DAG). The +// plugin resolves both the hash sidecar and the slim manifest by directory +// alone, with no check of which manifest produced them, so naming one via +// ManifestNames must NOT cause it to be stamped: the other DAG would then load +// this manifest's slim copy and hash. Nothing gets written for either file. +func TestRunSkipsAmbiguousDirectoryEvenWithOverride(t *testing.T) { root := t.TempDir() writeFiles(t, root, map[string]string{ "manifests_per_schedule/manifest_global_daily_schedule.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{"model.daily":{"name":"daily"}}}`, @@ -137,23 +141,26 @@ func TestRunHonorsManifestNameOverride(t *testing.T) { }) want := filepath.Join(root, "manifests_per_schedule", "manifest_full.json") - summary, err := Run([]string{root}, "test", Options{ManifestNames: map[string]string{"manifests_per_schedule": "manifest_full.json"}}) + summary, err := Run([]string{root}, "test", Options{SlimManifest: true, ManifestNames: map[string]string{"manifests_per_schedule": "manifest_full.json"}}) if err != nil { t.Fatal(err) } - if len(summary.Results) != 1 || summary.Results[0].Path != want || summary.Results[0].Err != nil { - t.Fatalf("want 1 result for manifest_full.json only, got %+v", summary.Results) + if len(summary.Results) != 1 || summary.Results[0].Path != want || !summary.Results[0].Skipped || summary.Results[0].Warning == "" { + t.Fatalf("want 1 skipped result with a reason, got %+v", summary.Results) + } + if _, err := os.Stat(filepath.Join(root, "manifests_per_schedule", sidecarDir)); !os.IsNotExist(err) { + t.Fatalf("an ambiguous directory must get no .astro/ at all: %v", err) } - mustExist(t, filepath.Join(root, "manifests_per_schedule", sidecarDir, sidecarName)) } // TestRunManifestNameOverrideExcludesDefaultName: a directory's override // replaces manifest.json rather than adding to it - a manifest.json sitting -// alongside the named file is left untouched. +// alongside the named file (here, not itself a dbt manifest, so it can't also +// trigger the ambiguity guard) is left untouched. func TestRunManifestNameOverrideExcludesDefaultName(t *testing.T) { root := t.TempDir() writeFiles(t, root, map[string]string{ - "shipped/manifest.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, + "shipped/manifest.json": `{"name":"My App","icons":[]}`, "shipped/manifest_full.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, }) want := filepath.Join(root, "shipped", "manifest_full.json") @@ -162,8 +169,36 @@ func TestRunManifestNameOverrideExcludesDefaultName(t *testing.T) { if err != nil { t.Fatal(err) } - if len(summary.Results) != 1 || summary.Results[0].Path != want { - t.Fatalf("want 1 result for manifest_full.json only, manifest.json must be ignored: %+v", summary.Results) + if len(summary.Results) != 1 || summary.Results[0].Path != want || summary.Results[0].Err != nil || summary.Results[0].Skipped { + t.Fatalf("want 1 stamped result for manifest_full.json only, manifest.json must be ignored: %+v", summary.Results) + } + mustExist(t, filepath.Join(root, "shipped", sidecarDir, sidecarName)) +} + +// TestRunSkipsSlimInProjectRootWhenAmbiguous: the processProject side of the +// same guard - a project root holding two valid dbt manifests still gets its +// own tree-hash sidecar (unaffected, since it isn't manifest-specific), but +// is not slimmed, since either manifest's DAG could be the wrong one to load +// the other's slim copy. +func TestRunSkipsSlimInProjectRootWhenAmbiguous(t *testing.T) { + root := t.TempDir() + writeFiles(t, root, map[string]string{ + "proj/dbt_project.yml": "name: shop\n", + "proj/models/a.sql": "select 1", + "proj/manifest.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, + "proj/manifest_full.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, + }) + + summary, err := Run([]string{root}, "test", Options{SlimManifest: true}) + if err != nil { + t.Fatal(err) + } + if len(summary.Results) != 1 || summary.Results[0].Kind != kindProject || summary.Results[0].Err != nil || summary.Results[0].Warning == "" { + t.Fatalf("want 1 project result with a warning, got %+v", summary.Results) + } + mustExist(t, filepath.Join(root, "proj", sidecarDir, sidecarName)) + if _, err := os.Stat(filepath.Join(root, "proj", sidecarDir, slimManifestName)); !os.IsNotExist(err) { + t.Fatalf("an ambiguous project root must not be slimmed: %v", err) } } From b91f537e31924ce05f656ce97561ce0f2e2075f0 Mon Sep 17 00:00:00 2001 From: Pankaj Singh Date: Tue, 29 Sep 2026 23:38:56 +0530 Subject: [PATCH 04/13] refactor: trim redundant checks, tests, and comments - effectiveManifestName: drop the len(overrides)==0 fast path - a nil map lookup already returns ("", false) safely, so it was dead code. - manifestNameOverrides: filepath.Base("") == "." != "", so the explicit name == "" check was redundant with the bare-filename check. - Inline joinWarnings (used at exactly one call site) instead of a standalone helper. - Drop TestRunManifestNameOverrideInProjectRootIsSlimmedNotDiscovered: fully subsumed by TestRunManifestNameOverridePerDirectory's dbt1 case. - Replace two PreDeploy-level rejection tests (each spinning up a tempdir to hit an error that happens before any filesystem access) with one direct TestManifestNameOverrides covering unset/valid/ invalid-JSON/path-valued cases against the parser itself. - Shrink processManifest's doc comment to a pointer at hasSiblingDbtManifest instead of restating its rationale. Co-Authored-By: Claude --- pkg/cosmosboost/precompute/discover.go | 3 -- pkg/cosmosboost/precompute/precompute.go | 24 +++++--------- pkg/cosmosboost/precompute/precompute_test.go | 27 +++------------- pkg/cosmosboost/predeploy.go | 2 +- pkg/cosmosboost/predeploy_test.go | 31 ++++++++++++------- 5 files changed, 31 insertions(+), 56 deletions(-) diff --git a/pkg/cosmosboost/precompute/discover.go b/pkg/cosmosboost/precompute/discover.go index 4767578b2..9ef67756d 100644 --- a/pkg/cosmosboost/precompute/discover.go +++ b/pkg/cosmosboost/precompute/discover.go @@ -56,9 +56,6 @@ var manifestSkipDirs = map[string]bool{ // where rel is dir's slash-separated path relative to root ("." for root // itself), or manifestFile when overrides is empty or has no entry for rel. func effectiveManifestName(overrides map[string]string, root, dir string) string { - if len(overrides) == 0 { - return manifestFile - } rel, err := filepath.Rel(root, dir) if err != nil { return manifestFile diff --git a/pkg/cosmosboost/precompute/precompute.go b/pkg/cosmosboost/precompute/precompute.go index bdd7870f0..84d57fd9d 100644 --- a/pkg/cosmosboost/precompute/precompute.go +++ b/pkg/cosmosboost/precompute/precompute.go @@ -168,7 +168,11 @@ func processProject(dir, root, version string, opts Options) Result { return r } if ambiguous { - r.Warning = joinWarnings(r.Warning, name+" not slimmed: another valid dbt manifest shares this directory") + note := name + " not slimmed: another valid dbt manifest shares this directory" + if r.Warning != "" { + note = r.Warning + "; " + note + } + r.Warning = note } else { // Nothing mutates doc afterward here, unlike processManifest. data, _ := json.Marshal(buildSlimManifest(doc, version)) @@ -188,13 +192,8 @@ func processProject(dir, root, version string, opts Options) Result { // processManifest hashes one manifest.json and writes a sidecar next to it, // plus a slim, field-filtered copy of the manifest when opts asks for one (see // buildSlimManifest). A file that isn't a dbt manifest is skipped (nothing is -// written) so unrelated manifest.json files in the project aren't stamped. -// -// It is also skipped when another valid dbt manifest shares its directory -// (see hasSiblingDbtManifest): both artifacts would land in the same .astro/, -// and the plugin has no way to tell which manifest they belong to, so writing -// them risks serving this manifest's slim copy and hash to a DAG that -// actually points at the sibling. +// written) so unrelated manifest.json files in the project aren't stamped - +// same as one with a sibling dbt manifest (see hasSiblingDbtManifest). func processManifest(path, version string, opts Options) Result { start := time.Now() doc, bytes, isDbt, err := readManifestDoc(path) @@ -238,15 +237,6 @@ func processManifest(path, version string, opts Options) Result { return r } -// joinWarnings appends add to existing, semicolon-separated, so a later -// warning doesn't overwrite an earlier one on the same Result. -func joinWarnings(existing, add string) string { - if existing == "" { - return add - } - return existing + "; " + add -} - // writeSlimManifest writes data as dir's slim manifest and returns the sidecar // pointer describing it. data must already be marshaled, so a caller that later // mutates the source doc cannot leak into it (see processManifest). diff --git a/pkg/cosmosboost/precompute/precompute_test.go b/pkg/cosmosboost/precompute/precompute_test.go index c36c4badc..8b19a3214 100644 --- a/pkg/cosmosboost/precompute/precompute_test.go +++ b/pkg/cosmosboost/precompute/precompute_test.go @@ -202,31 +202,12 @@ func TestRunSkipsSlimInProjectRootWhenAmbiguous(t *testing.T) { } } -// TestRunManifestNameOverrideInProjectRootIsSlimmedNotDiscovered: a -// ManifestNames file in a project root ("." - the project dir itself) is -// slimmed by processProject, not discovered as its own unit (same as -// manifest.json there). -func TestRunManifestNameOverrideInProjectRootIsSlimmedNotDiscovered(t *testing.T) { - root := t.TempDir() - writeFiles(t, root, map[string]string{ - "proj/dbt_project.yml": "name: shop\n", - "proj/models/a.sql": "select 1", - "proj/manifest_full.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json","project_name":"shop"},"nodes":{"model.shop.orders":{"original_file_path":"models/orders.sql","package_name":"shop","resource_type":"model","fqn":["shop","orders"]}}}`, - }) - - summary, err := Run([]string{root}, "test", Options{SlimManifest: true, ManifestNames: map[string]string{"proj": "manifest_full.json"}}) - if err != nil { - t.Fatal(err) - } - if len(summary.Results) != 1 || summary.Results[0].Kind != kindProject || summary.Results[0].Err != nil { - t.Fatalf("want 1 project-only result, got %+v", summary.Results) - } - mustExist(t, filepath.Join(root, "proj", sidecarDir, slimManifestName)) -} - // TestRunManifestNameOverridePerDirectory: two dbt projects with different // manifest-root conventions in one Run - ManifestNames applies each -// directory's own override independently. +// directory's own override independently. dbt1 also pins that a +// ManifestNames file in a project root is slimmed by processProject, not +// discovered as its own unit (same as manifest.json there): a stray third +// unit would make the result count 3, not 2. func TestRunManifestNameOverridePerDirectory(t *testing.T) { root := t.TempDir() writeFiles(t, root, map[string]string{ diff --git a/pkg/cosmosboost/predeploy.go b/pkg/cosmosboost/predeploy.go index 472a0cba6..a7bb10c26 100644 --- a/pkg/cosmosboost/predeploy.go +++ b/pkg/cosmosboost/predeploy.go @@ -50,7 +50,7 @@ func manifestNameOverrides() (map[string]string, error) { return nil, fmt.Errorf("%s must be a JSON object of directory to manifest filename: %w", manifestNameEnvVar, err) } for dir, name := range overrides { - if name == "" || filepath.Base(name) != name { + if filepath.Base(name) != name { return nil, fmt.Errorf("%s: %q for directory %q must be a bare filename, not a path", manifestNameEnvVar, name, dir) } } diff --git a/pkg/cosmosboost/predeploy_test.go b/pkg/cosmosboost/predeploy_test.go index 850b50652..46e5dc237 100644 --- a/pkg/cosmosboost/predeploy_test.go +++ b/pkg/cosmosboost/predeploy_test.go @@ -118,21 +118,28 @@ func TestPreDeployRespectsManifestNameEnvVar(t *testing.T) { require.FileExists(t, filepath.Join(dir, artifactRelPath)) } -// TestPreDeployRejectsInvalidManifestNameJSON: malformed JSON in the env var -// fails the deploy step rather than silently matching nothing. -func TestPreDeployRejectsInvalidManifestNameJSON(t *testing.T) { - dir := t.TempDir() +// TestManifestNameOverrides: unset leaves every directory matching +// manifest.json; malformed JSON or a value containing a path separator +// (which would never match findManifests' basename comparison) is rejected +// instead of silently matching nothing. +func TestManifestNameOverrides(t *testing.T) { + t.Setenv(manifestNameEnvVar, "") + got, err := manifestNameOverrides() + require.NoError(t, err) + require.Nil(t, got) + + t.Setenv(manifestNameEnvVar, `{".": "manifest_full.json"}`) + got, err = manifestNameOverrides() + require.NoError(t, err) + require.Equal(t, map[string]string{".": "manifest_full.json"}, got) + t.Setenv(manifestNameEnvVar, "not-json") - require.ErrorContains(t, PreDeploy(dir), manifestNameEnvVar) -} + _, err = manifestNameOverrides() + require.ErrorContains(t, err, manifestNameEnvVar) -// TestPreDeployRejectsPathValuedManifestName: a value containing a path -// separator would never match findManifests' basename comparison, so it's -// rejected up front instead of silently stamping nothing. -func TestPreDeployRejectsPathValuedManifestName(t *testing.T) { - dir := t.TempDir() t.Setenv(manifestNameEnvVar, `{".": "target/manifest_full.json"}`) - require.ErrorContains(t, PreDeploy(dir), manifestNameEnvVar) + _, err = manifestNameOverrides() + require.ErrorContains(t, err, manifestNameEnvVar) } func TestPreDeployNoDbtContentIsANoOp(t *testing.T) { From fe2fe0636180536a4d308a0b13b11abdb9220763 Mon Sep 17 00:00:00 2001 From: Pankaj Singh Date: Tue, 29 Sep 2026 23:45:59 +0530 Subject: [PATCH 05/13] refactor: trim comments to their essential point No behavior change. Doc comments added across this PR's earlier commits restated what the code already shows; cut each to the non-obvious "why" (or the keying/format convention where that isn't otherwise visible), dropping the rest. Co-Authored-By: Claude --- pkg/cosmosboost/precompute/discover.go | 10 +++--- pkg/cosmosboost/precompute/hash.go | 12 +++---- pkg/cosmosboost/precompute/precompute.go | 9 ++---- pkg/cosmosboost/precompute/precompute_test.go | 31 ++++++------------- pkg/cosmosboost/predeploy.go | 13 +++----- pkg/cosmosboost/predeploy_test.go | 11 +++---- 6 files changed, 29 insertions(+), 57 deletions(-) diff --git a/pkg/cosmosboost/precompute/discover.go b/pkg/cosmosboost/precompute/discover.go index 9ef67756d..525a09a22 100644 --- a/pkg/cosmosboost/precompute/discover.go +++ b/pkg/cosmosboost/precompute/discover.go @@ -52,9 +52,8 @@ var manifestSkipDirs = map[string]bool{ gitDir: true, // VCS internals can't hold a project's manifest } -// effectiveManifestName is the filename expected in dir: overrides[rel], -// where rel is dir's slash-separated path relative to root ("." for root -// itself), or manifestFile when overrides is empty or has no entry for rel. +// effectiveManifestName resolves dir's override in overrides (keyed by its +// path relative to root, "." for root itself), or manifestFile if none. func effectiveManifestName(overrides map[string]string, root, dir string) string { rel, err := filepath.Rel(root, dir) if err != nil { @@ -66,9 +65,8 @@ func effectiveManifestName(overrides map[string]string, root, dir string) string return manifestFile } -// findManifests walks root and returns file paths matching, in each -// directory, the name effectiveManifestName resolves for it - manifestFile -// by default, or a per-directory override. +// findManifests walks root and returns file paths matching each directory's +// effectiveManifestName. // // A manifest whose parent directory is itself a discovered project root is // omitted: that project's folder hash already covers a manifest sitting in its diff --git a/pkg/cosmosboost/precompute/hash.go b/pkg/cosmosboost/precompute/hash.go index b372f9ae0..5ca9e4ab9 100644 --- a/pkg/cosmosboost/precompute/hash.go +++ b/pkg/cosmosboost/precompute/hash.go @@ -211,14 +211,10 @@ func readManifestDoc(path string) (doc map[string]any, bytes int64, isDbt bool, return doc, bytes, true, nil } -// hasSiblingDbtManifest reports whether dir holds a *.json file, other than -// exclude, that also parses as a valid dbt manifest. The Cosmos Boost plugin -// resolves both the hash sidecar and the slim manifest by directory alone, -// with no check of which manifest produced them - so stamping dir while it -// holds more than one valid dbt manifest risks serving one manifest's -// artifacts to a DAG that points at the other. Checking .json files only -// keeps the cost bounded to plausible candidates in that single directory, -// not a tree-wide scan. +// hasSiblingDbtManifest reports whether dir holds another *.json file that +// also parses as a valid dbt manifest. The plugin resolves both artifacts by +// directory alone, not by manifest identity, so stamping an ambiguous +// directory risks serving one manifest's cache to a DAG pointed at the other. func hasSiblingDbtManifest(dir, exclude string) (bool, error) { entries, err := os.ReadDir(dir) if err != nil { diff --git a/pkg/cosmosboost/precompute/precompute.go b/pkg/cosmosboost/precompute/precompute.go index 84d57fd9d..0f4b6260d 100644 --- a/pkg/cosmosboost/precompute/precompute.go +++ b/pkg/cosmosboost/precompute/precompute.go @@ -46,12 +46,9 @@ type Options struct { // manifest.json (see buildSlimManifest) next to its sidecar. SlimManifest bool - // ManifestNames overrides, per directory, the filename findManifests and - // processProject match instead of manifest.json - for a manifest under a - // different name, or to pick one out of several valid manifests in the - // same directory. Keyed by a directory's slash-separated path relative to - // its root ("." for the root itself); a directory with no entry still - // uses manifest.json. + // ManifestNames overrides the filename matched in a directory (keyed by + // its path relative to its root, "." for the root itself) instead of + // manifest.json. ManifestNames map[string]string } diff --git a/pkg/cosmosboost/precompute/precompute_test.go b/pkg/cosmosboost/precompute/precompute_test.go index 8b19a3214..a2ea3eeb6 100644 --- a/pkg/cosmosboost/precompute/precompute_test.go +++ b/pkg/cosmosboost/precompute/precompute_test.go @@ -126,13 +126,9 @@ func TestRunSkipsNonDBTManifest(t *testing.T) { } } -// TestRunSkipsAmbiguousDirectoryEvenWithOverride: the real hazard this PR's -// override could otherwise hit head-on - two valid dbt manifests share a -// directory (e.g. per-schedule manifests, each read by a different DAG). The -// plugin resolves both the hash sidecar and the slim manifest by directory -// alone, with no check of which manifest produced them, so naming one via -// ManifestNames must NOT cause it to be stamped: the other DAG would then load -// this manifest's slim copy and hash. Nothing gets written for either file. +// TestRunSkipsAmbiguousDirectoryEvenWithOverride: naming one of two valid dbt +// manifests via ManifestNames must not stamp it - the plugin resolves both +// artifacts by directory alone, so the other manifest's DAG would load them. func TestRunSkipsAmbiguousDirectoryEvenWithOverride(t *testing.T) { root := t.TempDir() writeFiles(t, root, map[string]string{ @@ -153,10 +149,8 @@ func TestRunSkipsAmbiguousDirectoryEvenWithOverride(t *testing.T) { } } -// TestRunManifestNameOverrideExcludesDefaultName: a directory's override -// replaces manifest.json rather than adding to it - a manifest.json sitting -// alongside the named file (here, not itself a dbt manifest, so it can't also -// trigger the ambiguity guard) is left untouched. +// TestRunManifestNameOverrideExcludesDefaultName: an override replaces +// manifest.json rather than adding to it. func TestRunManifestNameOverrideExcludesDefaultName(t *testing.T) { root := t.TempDir() writeFiles(t, root, map[string]string{ @@ -175,11 +169,8 @@ func TestRunManifestNameOverrideExcludesDefaultName(t *testing.T) { mustExist(t, filepath.Join(root, "shipped", sidecarDir, sidecarName)) } -// TestRunSkipsSlimInProjectRootWhenAmbiguous: the processProject side of the -// same guard - a project root holding two valid dbt manifests still gets its -// own tree-hash sidecar (unaffected, since it isn't manifest-specific), but -// is not slimmed, since either manifest's DAG could be the wrong one to load -// the other's slim copy. +// TestRunSkipsSlimInProjectRootWhenAmbiguous: a project root with two valid +// dbt manifests still gets its tree-hash sidecar, but isn't slimmed. func TestRunSkipsSlimInProjectRootWhenAmbiguous(t *testing.T) { root := t.TempDir() writeFiles(t, root, map[string]string{ @@ -202,12 +193,8 @@ func TestRunSkipsSlimInProjectRootWhenAmbiguous(t *testing.T) { } } -// TestRunManifestNameOverridePerDirectory: two dbt projects with different -// manifest-root conventions in one Run - ManifestNames applies each -// directory's own override independently. dbt1 also pins that a -// ManifestNames file in a project root is slimmed by processProject, not -// discovered as its own unit (same as manifest.json there): a stray third -// unit would make the result count 3, not 2. +// TestRunManifestNameOverridePerDirectory: two projects with different +// manifest-root conventions in one Run each apply their own override. func TestRunManifestNameOverridePerDirectory(t *testing.T) { root := t.TempDir() writeFiles(t, root, map[string]string{ diff --git a/pkg/cosmosboost/predeploy.go b/pkg/cosmosboost/predeploy.go index a7bb10c26..d680790b1 100644 --- a/pkg/cosmosboost/predeploy.go +++ b/pkg/cosmosboost/predeploy.go @@ -30,16 +30,13 @@ func slimManifestEnabled() bool { return value == "" || util.CheckEnvBool(value) } -// manifestNameEnvVar holds a JSON object mapping a directory (relative to the -// deployed path, "." for its root) to the filename discovery should match -// there instead of manifest.json (see precompute.Options.ManifestNames). A -// directory with no entry keeps matching manifest.json. +// manifestNameEnvVar holds a JSON directory->filename map (see +// precompute.Options.ManifestNames). const manifestNameEnvVar = "ASTRO_COSMOS_BOOST_MANIFEST_NAME" -// manifestNameOverrides parses manifestNameEnvVar, or returns nil when unset -// (every directory keeps matching manifest.json). Every value must be a bare -// filename (no path separators): findManifests matches on a file's basename -// alone, so a path there would silently match nothing. +// manifestNameOverrides parses manifestNameEnvVar ("" when unset). Every +// value must be a bare filename: findManifests matches by basename, so a +// path there would silently match nothing. func manifestNameOverrides() (map[string]string, error) { value := strings.TrimSpace(os.Getenv(manifestNameEnvVar)) if value == "" { diff --git a/pkg/cosmosboost/predeploy_test.go b/pkg/cosmosboost/predeploy_test.go index 46e5dc237..473dcbc44 100644 --- a/pkg/cosmosboost/predeploy_test.go +++ b/pkg/cosmosboost/predeploy_test.go @@ -101,9 +101,8 @@ func TestSlimManifestEnabled(t *testing.T) { } } -// TestPreDeployRespectsManifestNameEnvVar: a manifest.json-only discovery -// misses a differently-named file, but stamps it once the env var names it -// for the deploy root ("."). +// TestPreDeployRespectsManifestNameEnvVar: a differently-named manifest is +// stamped once the env var names it for the deploy root ("."). func TestPreDeployRespectsManifestNameEnvVar(t *testing.T) { dir := t.TempDir() manifest := `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}` @@ -118,10 +117,8 @@ func TestPreDeployRespectsManifestNameEnvVar(t *testing.T) { require.FileExists(t, filepath.Join(dir, artifactRelPath)) } -// TestManifestNameOverrides: unset leaves every directory matching -// manifest.json; malformed JSON or a value containing a path separator -// (which would never match findManifests' basename comparison) is rejected -// instead of silently matching nothing. +// TestManifestNameOverrides: unset is nil; malformed JSON or a non-bare +// filename value is rejected instead of silently matching nothing. func TestManifestNameOverrides(t *testing.T) { t.Setenv(manifestNameEnvVar, "") got, err := manifestNameOverrides() From 9bcabe82650a0eba01c2dd1a4bb89a03dc71fdf2 Mon Sep 17 00:00:00 2001 From: Pankaj Singh Date: Wed, 30 Sep 2026 00:01:24 +0530 Subject: [PATCH 06/13] fix: don't flag other dbt artifacts as an ambiguous sibling manifest isDbtManifest checked only for metadata.dbt_schema_version's presence, which every dbt artifact carries (run_results.json, catalog.json, sources.json, semantic_manifest.json - not just manifest.json), each under its own schema URL. On main this was harmless: it only ever ran on a file already found by exact filename match. hasSiblingDbtManifest breaks that assumption by scanning every *.json in a directory by content - so a plain "dbt build" (which always writes run_results.json next to manifest.json in target/) would flag the directory ambiguous and silently stop stamping manifest.json for most real projects. Tightened to require the schema URL contain "/manifest/", which every dbt manifest schema does and no other dbt artifact's does (confirmed against schemas.getdbt.com). Also fixed two related error-handling gaps found in the same pass: processProject aborted the whole unit (skipping its always-safe tree-hash sidecar) when the sibling check itself failed to read the directory, instead of just skipping the slim attachment the way an actually-ambiguous directory does; and processManifest promoted that same read failure to a hard Err instead of the same fail-safe Skipped+Warning an ambiguous directory gets. Co-Authored-By: Claude --- pkg/cosmosboost/precompute/hash.go | 10 +++-- pkg/cosmosboost/precompute/hash_test.go | 18 +++++--- pkg/cosmosboost/precompute/precompute.go | 45 +++++++++++-------- pkg/cosmosboost/precompute/precompute_test.go | 32 +++++++++++++ 4 files changed, 75 insertions(+), 30 deletions(-) diff --git a/pkg/cosmosboost/precompute/hash.go b/pkg/cosmosboost/precompute/hash.go index 5ca9e4ab9..f948a882a 100644 --- a/pkg/cosmosboost/precompute/hash.go +++ b/pkg/cosmosboost/precompute/hash.go @@ -10,6 +10,7 @@ import ( "os" "path/filepath" "sort" + "strings" ) // excludedDirs are directory names skipped during *project* discovery @@ -177,16 +178,17 @@ func stripEntityCreatedAt(doc map[string]any) { } } -// isDbtManifest reports whether doc looks like a dbt manifest. dbt always writes -// metadata.dbt_schema_version, which unrelated manifest.json files (web app/PWA, -// tooling, etc.) do not have — so we only stamp files that carry it. +// isDbtManifest reports whether doc is a dbt manifest specifically - every +// dbt artifact (run_results.json, catalog.json, ...) carries the same +// metadata.dbt_schema_version, just under its own schema URL, so presence +// alone would also match run_results.json, which target/ always has too. func isDbtManifest(doc map[string]any) bool { meta, ok := doc["metadata"].(map[string]any) if !ok { return false } v, ok := meta["dbt_schema_version"].(string) - return ok && v != "" + return ok && strings.Contains(v, "/manifest/") } // readManifestDoc reads path and parses it as JSON. isDbt reports whether the diff --git a/pkg/cosmosboost/precompute/hash_test.go b/pkg/cosmosboost/precompute/hash_test.go index 8d3b7dd04..f3af1c7b9 100644 --- a/pkg/cosmosboost/precompute/hash_test.go +++ b/pkg/cosmosboost/precompute/hash_test.go @@ -220,15 +220,19 @@ func TestHashManifestIgnoresVolatileMetadata(t *testing.T) { } } -// TestHashManifestSkipsNonDBT verifies that a manifest.json lacking the dbt shape -// (e.g. a web-app/PWA manifest) or invalid JSON is not treated as a dbt manifest, -// so it won't be stamped. +// TestHashManifestSkipsNonDBT verifies that a manifest.json lacking the dbt +// shape (e.g. a web-app/PWA manifest), invalid JSON, or another dbt artifact +// entirely (run_results.json, catalog.json - both carry the same +// metadata.dbt_schema_version field dbt manifests do) is not treated as a dbt +// manifest, so it won't be stamped. func TestHashManifestSkipsNonDBT(t *testing.T) { dir := t.TempDir() cases := map[string]string{ - "webapp.json": `{"name":"My App","short_name":"App","icons":[]}`, // no metadata.dbt_schema_version - "nometa.json": `{"nodes":{"model.x":{}}}`, // nodes but no metadata - "invalid.json": `{not json`, // not JSON at all + "webapp.json": `{"name":"My App","short_name":"App","icons":[]}`, // no metadata.dbt_schema_version + "nometa.json": `{"nodes":{"model.x":{}}}`, // nodes but no metadata + "invalid.json": `{not json`, // not JSON at all + "run_results.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/run-results/v6.json"}}`, // a dbt artifact, but not the manifest + "catalog.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/catalog/v1.json"},"nodes":{}}`, // ditto, and it even has "nodes" } for name, content := range cases { p := filepath.Join(dir, name) @@ -484,7 +488,7 @@ func TestHashManifestStableAcrossFullParses(t *testing.T) { func TestHashManifestKeepsUserMetaCreatedAt(t *testing.T) { // The scalar top-level key pins that the created_at stripper tolerates // non-collection values in the document root. - base := `{"metadata": {"dbt_schema_version": "v12"}, "unrelated_scalar": 7, + base := `{"metadata": {"dbt_schema_version": "https://schemas.getdbt.com/dbt/manifest/v12.json"}, "unrelated_scalar": 7, "nodes": {"model.shop.a": {"name": "a", "created_at": 1.0, "meta": {"created_at": "%s"}}}}` dir := t.TempDir() writeFiles(t, dir, map[string]string{ diff --git a/pkg/cosmosboost/precompute/precompute.go b/pkg/cosmosboost/precompute/precompute.go index 0f4b6260d..6fa511a72 100644 --- a/pkg/cosmosboost/precompute/precompute.go +++ b/pkg/cosmosboost/precompute/precompute.go @@ -156,21 +156,17 @@ func processProject(dir, root, version string, opts Options) Result { if opts.SlimManifest { name := effectiveManifestName(opts.ManifestNames, root, dir) if doc, _, isDbt, readErr := readManifestDoc(filepath.Join(dir, name)); readErr == nil && isDbt { - // Skip slimming (not the whole project) when another valid dbt - // manifest shares this directory: see hasSiblingDbtManifest. - ambiguous, sibErr := hasSiblingDbtManifest(dir, name) - if sibErr != nil { - r.Err = sibErr - r.Duration = time.Since(start) - return r - } - if ambiguous { - note := name + " not slimmed: another valid dbt manifest shares this directory" - if r.Warning != "" { - note = r.Warning + "; " + note - } - r.Warning = note - } else { + // A failed ambiguity check only skips slimming, not the whole + // project: unlike a writeSlimManifest failure below, it leaves no + // partial artifact, and the project's tree hash below doesn't + // depend on it either way. + note := "" + switch ambiguous, sibErr := hasSiblingDbtManifest(dir, name); { + case sibErr != nil: + note = name + " not slimmed: could not check for another manifest in this directory (" + sibErr.Error() + ")" + case ambiguous: + note = name + " not slimmed: another valid dbt manifest shares this directory" + default: // Nothing mutates doc afterward here, unlike processManifest. data, _ := json.Marshal(buildSlimManifest(doc, version)) if filtered, r.Err = writeSlimManifest(dir, data); r.Err != nil { @@ -178,6 +174,12 @@ func processProject(dir, root, version string, opts Options) Result { return r } } + if note != "" { + if r.Warning != "" { + note = r.Warning + "; " + note + } + r.Warning = note + } } } @@ -196,9 +198,14 @@ func processManifest(path, version string, opts Options) Result { doc, bytes, isDbt, err := readManifestDoc(path) var hash string var slimData []byte - var ambiguous bool + var skipNote string if err == nil && isDbt { - if ambiguous, err = hasSiblingDbtManifest(filepath.Dir(path), filepath.Base(path)); err == nil && !ambiguous { + switch ambiguous, sibErr := hasSiblingDbtManifest(filepath.Dir(path), filepath.Base(path)); { + case sibErr != nil: + skipNote = "could not check for another manifest in this directory (" + sibErr.Error() + ")" + case ambiguous: + skipNote = "another valid dbt manifest shares this directory; skipped to avoid stamping the wrong one's cache" + default: if opts.SlimManifest { // Marshal before hashDocument mutates doc: the slim manifest shares // doc's nested values, so only turning it into bytes here decouples @@ -214,9 +221,9 @@ func processManifest(path, version string, opts Options) Result { r.Err = err case !isDbt: r.Skipped = true - case ambiguous: + case skipNote != "": r.Skipped = true - r.Warning = "another valid dbt manifest shares this directory; skipped to avoid stamping the wrong one's cache" + r.Warning = skipNote default: dir := filepath.Dir(path) // The sidecar goes last: it carries the filtered_manifest pointer, so it diff --git a/pkg/cosmosboost/precompute/precompute_test.go b/pkg/cosmosboost/precompute/precompute_test.go index a2ea3eeb6..c4aff267f 100644 --- a/pkg/cosmosboost/precompute/precompute_test.go +++ b/pkg/cosmosboost/precompute/precompute_test.go @@ -149,6 +149,38 @@ func TestRunSkipsAmbiguousDirectoryEvenWithOverride(t *testing.T) { } } +// TestRunDoesNotFlagOtherDbtArtifactsAsAmbiguous: the ordinary case of a +// compiled project - target/manifest.json sitting beside target/run_results.json +// and target/catalog.json, which "dbt build" and "dbt docs generate" always +// produce - must not trip hasSiblingDbtManifest. Regression test for a false +// positive that would have silently disabled stamping for most real projects. +func TestRunDoesNotFlagOtherDbtArtifactsAsAmbiguous(t *testing.T) { + root := t.TempDir() + writeFiles(t, root, map[string]string{ + "proj/dbt_project.yml": "name: shop\n", + "proj/models/a.sql": "select 1", + "proj/target/manifest.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, + "proj/target/run_results.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/run-results/v6.json"}}`, + "proj/target/catalog.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/catalog/v1.json"},"nodes":{}}`, + }) + + summary, err := Run([]string{root}, "test", Options{SlimManifest: true}) + if err != nil { + t.Fatal(err) + } + kinds := map[string]int{} + for _, r := range summary.Results { + if r.Err != nil || r.Skipped { + t.Fatalf("unexpected non-success result: %+v", r) + } + kinds[r.Kind]++ + } + if kinds["project"] != 1 || kinds["manifest"] != 1 { + t.Fatalf("want 1 project + 1 manifest, got %+v (%+v)", kinds, summary.Results) + } + mustExist(t, filepath.Join(root, "proj", "target", sidecarDir, slimManifestName)) +} + // TestRunManifestNameOverrideExcludesDefaultName: an override replaces // manifest.json rather than adding to it. func TestRunManifestNameOverrideExcludesDefaultName(t *testing.T) { From ced66e787bfae5b1d1451d418488b046bd6f722b Mon Sep 17 00:00:00 2001 From: Pankaj Singh Date: Wed, 30 Sep 2026 00:16:00 +0530 Subject: [PATCH 07/13] refactor: drop unreachable error branch in effectiveManifestName dir always descends from root (it only ever comes from walking root), so filepath.Rel(root, dir) can't realistically fail here - and even if it did, Rel returns "" on error, which already can't match a real override key (the documented convention is "." for root). The explicit early-return added nothing a plain fallthrough doesn't already do correctly. Co-Authored-By: Claude --- pkg/cosmosboost/precompute/discover.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/cosmosboost/precompute/discover.go b/pkg/cosmosboost/precompute/discover.go index 525a09a22..60f6ab567 100644 --- a/pkg/cosmosboost/precompute/discover.go +++ b/pkg/cosmosboost/precompute/discover.go @@ -55,10 +55,7 @@ var manifestSkipDirs = map[string]bool{ // effectiveManifestName resolves dir's override in overrides (keyed by its // path relative to root, "." for root itself), or manifestFile if none. func effectiveManifestName(overrides map[string]string, root, dir string) string { - rel, err := filepath.Rel(root, dir) - if err != nil { - return manifestFile - } + rel, _ := filepath.Rel(root, dir) // dir always descends from root; an error here can't match a real override key if name, ok := overrides[filepath.ToSlash(rel)]; ok && name != "" { return name } From c6117a64af4cbe53599b6a06b54ec054e4bd9476 Mon Sep 17 00:00:00 2001 From: Pankaj Singh Date: Wed, 30 Sep 2026 00:22:01 +0530 Subject: [PATCH 08/13] test: pin ManifestNames override for a dbt project nested under dags/ The override key is a path relative to the whole deploy root, not to the project's own directory - worth pinning since a dbt project living under dags/ (dags/dbt//) is a common Astro layout, and it would be easy to assume the key is just the project's own folder name. Co-Authored-By: Claude --- pkg/cosmosboost/precompute/precompute_test.go | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/pkg/cosmosboost/precompute/precompute_test.go b/pkg/cosmosboost/precompute/precompute_test.go index c4aff267f..36fba714c 100644 --- a/pkg/cosmosboost/precompute/precompute_test.go +++ b/pkg/cosmosboost/precompute/precompute_test.go @@ -201,6 +201,27 @@ func TestRunManifestNameOverrideExcludesDefaultName(t *testing.T) { mustExist(t, filepath.Join(root, "shipped", sidecarDir, sidecarName)) } +// TestRunManifestNameOverrideForNestedProject: a dbt project living under +// dags/ (a common Astro layout) is keyed by its full path relative to the +// deploy root, not by its own name alone. +func TestRunManifestNameOverrideForNestedProject(t *testing.T) { + root := t.TempDir() + writeFiles(t, root, map[string]string{ + "dags/dbt/shop/dbt_project.yml": "name: shop\n", + "dags/dbt/shop/models/a.sql": "select 1", + "dags/dbt/shop/manifest_custom.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json","project_name":"shop"},"nodes":{"model.shop.a":{"original_file_path":"models/a.sql","package_name":"shop","resource_type":"model","fqn":["shop","a"]}}}`, + }) + + summary, err := Run([]string{root}, "test", Options{SlimManifest: true, ManifestNames: map[string]string{"dags/dbt/shop": "manifest_custom.json"}}) + if err != nil { + t.Fatal(err) + } + if len(summary.Results) != 1 || summary.Results[0].Kind != kindProject || summary.Results[0].Err != nil { + t.Fatalf("want 1 project-only result, got %+v", summary.Results) + } + mustExist(t, filepath.Join(root, "dags", "dbt", "shop", sidecarDir, slimManifestName)) +} + // TestRunSkipsSlimInProjectRootWhenAmbiguous: a project root with two valid // dbt manifests still gets its tree-hash sidecar, but isn't slimmed. func TestRunSkipsSlimInProjectRootWhenAmbiguous(t *testing.T) { From a82ea27aacd6956203f24015e585ff2c939a2ca8 Mon Sep 17 00:00:00 2001 From: Pankaj Singh Date: Wed, 30 Sep 2026 16:35:55 +0530 Subject: [PATCH 09/13] refactor: replace env var override with automatic manifest discovery Rewritten from scratch, not incrementally: discovery no longer relies on any customer-configured filename. A file is treated as a manifest when its name contains "manifest" (case-insensitive) and its content validates as one (isDbtManifest) - covering manifest.json, manifest_full.json, manifest_by_schedule.json, etc. with nothing to configure. ASTRO_COSMOS_BOOST_MANIFEST_NAME, Options.ManifestNames, effectiveManifestName, and hasSiblingDbtManifest are all removed. Every manifest found now gets its own slim companion, named after itself (slimNameFor: "manifest_full.json" -> "manifest_full.slim.json", "manifest.json" -> "manifest.slim.json" as before) - so N manifests in one directory produce N slim files, never a single contested one. A project root with multiple manifests slims each in turn. Cleanup matches slim files by their .slim.json suffix instead of one fixed name, so it can find and remove any of them; ownership is still checked per file via its own _generated_by marker. .astro/dbt_metadata.json is untouched - same fixed name, same schema, same writer - so a directory with multiple manifests still gets one sidecar, written by whichever unit's processing finishes last. Co-Authored-By: Claude --- pkg/cosmosboost/precompute/cleanup.go | 16 +- pkg/cosmosboost/precompute/cleanup_test.go | 51 ++++++ pkg/cosmosboost/precompute/discover.go | 35 ++-- pkg/cosmosboost/precompute/hash.go | 32 +--- pkg/cosmosboost/precompute/precompute.go | 158 ++++++++--------- pkg/cosmosboost/precompute/precompute_test.go | 165 +++++++++--------- pkg/cosmosboost/precompute/slim.go | 17 +- pkg/cosmosboost/precompute/slim_test.go | 19 ++ pkg/cosmosboost/predeploy.go | 37 +--- pkg/cosmosboost/predeploy_test.go | 35 +--- 10 files changed, 275 insertions(+), 290 deletions(-) diff --git a/pkg/cosmosboost/precompute/cleanup.go b/pkg/cosmosboost/precompute/cleanup.go index c50c64871..586f23563 100644 --- a/pkg/cosmosboost/precompute/cleanup.go +++ b/pkg/cosmosboost/precompute/cleanup.go @@ -7,6 +7,7 @@ import ( "io/fs" "os" "path/filepath" + "strings" "time" ) @@ -39,8 +40,9 @@ type CleanupSummary struct { } // Cleanup removes every .astro/dbt_metadata.json sidecar and every -// .astro/manifest.slim.json under the given roots that this tool wrote, -// pruning each containing .astro directory when removal leaves it empty. +// .astro/*.slim.json slim manifest under the given roots that this tool +// wrote, pruning each containing .astro directory when removal leaves it +// empty. // // Each file is judged by its own producer marker, never by its neighbor's, // because the mixed states are real: a slim manifest outlives its sidecar when @@ -70,10 +72,10 @@ func Cleanup(roots []string) (CleanupSummary, error) { if filepath.Base(filepath.Dir(path)) != sidecarDir { return nil } - switch d.Name() { - case sidecarName: + switch { + case d.Name() == sidecarName: recordOnce(seen, &results, path, removeSidecar) - case slimManifestName: + case strings.HasSuffix(d.Name(), slimManifestSuffix): recordOnce(seen, &results, path, removeSlimManifest) } return nil @@ -143,8 +145,8 @@ func removeSidecar(path string) CleanupResult { }) } -// removeSlimManifest removes one .astro/manifest.slim.json, keeping it unless -// its own _generated_by marker names this tool. +// removeSlimManifest removes one .astro/*.slim.json, keeping it unless its +// own _generated_by marker names this tool. func removeSlimManifest(path string) CleanupResult { return removeArtifact(path, func(data []byte) bool { var marker struct { diff --git a/pkg/cosmosboost/precompute/cleanup_test.go b/pkg/cosmosboost/precompute/cleanup_test.go index ab25e75aa..181ecaf73 100644 --- a/pkg/cosmosboost/precompute/cleanup_test.go +++ b/pkg/cosmosboost/precompute/cleanup_test.go @@ -96,6 +96,57 @@ func TestCleanupRemovesSlimManifestAlongsideSidecar(t *testing.T) { } } +// TestCleanupRemovesCustomNamedSlimManifest: Cleanup matches by the +// .slim.json suffix, not a fixed literal name. +func TestCleanupRemovesCustomNamedSlimManifest(t *testing.T) { + dir := t.TempDir() + writeFiles(t, dir, map[string]string{ + "manifest_full.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, + }) + summary, err := Run([]string{dir}, "test", Options{SlimManifest: true}) + if err != nil || summary.CountFailed() > 0 { + t.Fatalf("stamping fixture manifest failed: err=%v failed=%d", err, summary.CountFailed()) + } + slimPath := filepath.Join(dir, sidecarDir, "manifest_full.slim.json") + if _, err := os.Stat(slimPath); err != nil { + t.Fatalf("fixture setup: slim manifest not written: %v", err) + } + + cleanupSummary, err := Cleanup([]string{dir}) + if err != nil { + t.Fatalf("Cleanup: %v", err) + } + if got := len(cleanupSummary.Results); got != 2 { + t.Fatalf("results = %d, want 2 (sidecar + slim manifest)", got) + } + if cleanupSummary.CountFailed() != 0 || cleanupSummary.CountKept() != 0 { + t.Fatalf("failed=%d kept=%d, want 0/0", cleanupSummary.CountFailed(), cleanupSummary.CountKept()) + } + if _, err := os.Stat(slimPath); !os.IsNotExist(err) { + t.Fatalf("custom-named slim manifest still present after cleanup: %v", err) + } +} + +// TestCleanupKeepsForeignSlimJSONSuffixedFile: the broader suffix match must +// not weaken the ownership check for a *.slim.json file we didn't write. +func TestCleanupKeepsForeignSlimJSONSuffixedFile(t *testing.T) { + dir := t.TempDir() + writeFiles(t, dir, map[string]string{ + ".astro/some_other_tool.slim.json": `{"_generated_by": {"application": "someone-else"}}`, + }) + + summary, err := Cleanup([]string{dir}) + if err != nil { + t.Fatalf("Cleanup: %v", err) + } + if got := summary.CountKept(); got != 1 { + t.Fatalf("kept = %d, want 1", got) + } + if _, err := os.Stat(filepath.Join(dir, sidecarDir, "some_other_tool.slim.json")); err != nil { + t.Fatalf("foreign *.slim.json file was removed: %v", err) + } +} + // TestCleanupJudgesEachArtifactSeparately: provenance is read per file, never // inferred from the neighbor. Deleting on the neighbor's marker would destroy // a file we do not own in one direction, and strand a stale artifact of ours - diff --git a/pkg/cosmosboost/precompute/discover.go b/pkg/cosmosboost/precompute/discover.go index 60f6ab567..3a7ef4f93 100644 --- a/pkg/cosmosboost/precompute/discover.go +++ b/pkg/cosmosboost/precompute/discover.go @@ -4,14 +4,13 @@ import ( "io/fs" "os" "path/filepath" + "strings" ) // dbt requires the project file to be named exactly dbt_project.yml (the .yaml // extension is not accepted), so we match only this name. const dbtProjectFile = "dbt_project.yml" -const manifestFile = "manifest.json" - // findProjects walks root and returns every directory that contains a // dbt_project.yml. // @@ -52,25 +51,27 @@ var manifestSkipDirs = map[string]bool{ gitDir: true, // VCS internals can't hold a project's manifest } -// effectiveManifestName resolves dir's override in overrides (keyed by its -// path relative to root, "." for root itself), or manifestFile if none. -func effectiveManifestName(overrides map[string]string, root, dir string) string { - rel, _ := filepath.Rel(root, dir) // dir always descends from root; an error here can't match a real override key - if name, ok := overrides[filepath.ToSlash(rel)]; ok && name != "" { - return name - } - return manifestFile +// isManifestCandidateName reports whether name could be a dbt manifest, by +// name alone: a *.json file whose name contains "manifest" (case-insensitive) +// - covers manifest.json, manifest_full.json, manifest_by_schedule.json, etc. +// without needing a customer to configure anything. Content is validated +// separately (isDbtManifest) before anything gets stamped, which is what +// keeps this from also matching e.g. semantic_manifest.json. +func isManifestCandidateName(name string) bool { + lower := strings.ToLower(name) + return strings.HasSuffix(lower, ".json") && strings.Contains(lower, "manifest") } -// findManifests walks root and returns file paths matching each directory's -// effectiveManifestName. +// findManifests walks root and returns every file matching +// isManifestCandidateName. // // A manifest whose parent directory is itself a discovered project root is // omitted: that project's folder hash already covers a manifest sitting in its -// root. Manifests elsewhere — most importantly a standalone one shipped for a -// manifest-only (DBT_MANIFEST) deployment, or a project's target/manifest.json — -// each get their own sidecar. -func findManifests(root string, projectDirs map[string]bool, overrides map[string]string) ([]string, error) { +// root, and processProject slims it there instead. Manifests elsewhere — +// most importantly a standalone one shipped for a manifest-only +// (DBT_MANIFEST) deployment, or a project's target/manifest.json — each get +// their own sidecar. +func findManifests(root string, projectDirs map[string]bool) ([]string, error) { var manifests []string err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { @@ -83,7 +84,7 @@ func findManifests(root string, projectDirs map[string]bool, overrides map[strin } return nil } - if d.Name() != effectiveManifestName(overrides, root, filepath.Dir(path)) { + if !isManifestCandidateName(d.Name()) { return nil } if projectDirs[filepath.Dir(path)] { diff --git a/pkg/cosmosboost/precompute/hash.go b/pkg/cosmosboost/precompute/hash.go index f948a882a..f4e6fd622 100644 --- a/pkg/cosmosboost/precompute/hash.go +++ b/pkg/cosmosboost/precompute/hash.go @@ -179,9 +179,11 @@ func stripEntityCreatedAt(doc map[string]any) { } // isDbtManifest reports whether doc is a dbt manifest specifically - every -// dbt artifact (run_results.json, catalog.json, ...) carries the same -// metadata.dbt_schema_version, just under its own schema URL, so presence -// alone would also match run_results.json, which target/ always has too. +// dbt artifact (run_results.json, catalog.json, semantic_manifest.json, ...) +// carries the same metadata.dbt_schema_version, just under its own schema +// URL, so presence alone would also match those - most importantly +// semantic_manifest.json, whose name (like discovery's) also contains +// "manifest". func isDbtManifest(doc map[string]any) bool { meta, ok := doc["metadata"].(map[string]any) if !ok { @@ -213,30 +215,6 @@ func readManifestDoc(path string) (doc map[string]any, bytes int64, isDbt bool, return doc, bytes, true, nil } -// hasSiblingDbtManifest reports whether dir holds another *.json file that -// also parses as a valid dbt manifest. The plugin resolves both artifacts by -// directory alone, not by manifest identity, so stamping an ambiguous -// directory risks serving one manifest's cache to a DAG pointed at the other. -func hasSiblingDbtManifest(dir, exclude string) (bool, error) { - entries, err := os.ReadDir(dir) - if err != nil { - return false, err - } - for _, e := range entries { - if e.IsDir() || e.Name() == exclude || filepath.Ext(e.Name()) != ".json" { - continue - } - _, _, isDbt, err := readManifestDoc(filepath.Join(dir, e.Name())) - if err != nil { - return false, err - } - if isDbt { - return true, nil - } - } - return false, nil -} - // hashDocument computes the "sha256-manifest-v2" hash of an already-parsed // dbt manifest document, after removing the volatile fields dbt regenerates on // every full parse, so the value is stable across recompiles of unchanged diff --git a/pkg/cosmosboost/precompute/precompute.go b/pkg/cosmosboost/precompute/precompute.go index 6fa511a72..f9207012b 100644 --- a/pkg/cosmosboost/precompute/precompute.go +++ b/pkg/cosmosboost/precompute/precompute.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "io" + "os" "path/filepath" "runtime" "sort" @@ -19,17 +20,17 @@ const ( ) // Result records what happened for one unit of work: either a dbt project -// directory or a standalone manifest.json. +// directory or a standalone manifest. type Result struct { Kind string // "project" or "manifest" - Path string // project directory, or manifest.json path + Path string // project directory, or manifest path Hash string // version hash (empty if Err != nil or Skipped) Files int // files hashed (1 for a manifest) Bytes int64 // total bytes hashed Duration time.Duration // time spent on this unit - Skipped bool // no sidecar written: not a dbt manifest, or (see Warning) an ambiguous directory - Warning string // non-fatal note, e.g. an unresolved template, or why a Skipped unit was skipped - Err error // non-nil if hashing, writing the sidecar, or writing the slim manifest failed + Skipped bool // a manifest-like file that isn't a dbt manifest (no sidecar written) + Warning string // non-fatal note (sidecar still written), e.g. an unresolved template + Err error // non-nil if hashing, writing the sidecar, or writing a slim manifest failed } // Summary is the structured outcome of a precompute run. It backs both the @@ -43,36 +44,31 @@ type Summary struct { // hash sidecars. type Options struct { // SlimManifest also writes a slim, field-filtered copy of each discovered - // manifest.json (see buildSlimManifest) next to its sidecar. + // manifest (see buildSlimManifest) next to its sidecar. SlimManifest bool - - // ManifestNames overrides the filename matched in a directory (keyed by - // its path relative to its root, "." for the root itself) instead of - // manifest.json. - ManifestNames map[string]string } -// Run finds every dbt project (a directory with dbt_project.yml) and standalone -// dbt manifest.json under the given roots, and writes a .astro/dbt_metadata.json -// hash sidecar next to each. Units are processed concurrently — one worker each, -// bounded by GOMAXPROCS — and each is hashed over sorted input, so results are -// deterministic with no cross-worker coordination. +// Run finds every dbt project (a directory with dbt_project.yml) and every +// standalone dbt manifest under the given roots, and writes a +// .astro/dbt_metadata.json hash sidecar next to each. Units are processed +// concurrently — one worker each, bounded by GOMAXPROCS — and each is hashed +// over sorted input, so results are deterministic with no cross-worker +// coordination. // // Per-unit failures are best-effort: a unit that fails is recorded in its Result // and does not stop the others. Run only returns a non-nil error for a top-level // problem, such as a root that cannot be walked. version is recorded in each // sidecar's generated_by. // -// With opts.SlimManifest set, every dbt manifest.json also gets a slim, -// field-filtered copy (see buildSlimManifest) written into the .astro/ beside -// it, for the Cosmos Boost plugin to load in place of the full manifest at -// DAG-parse time. That includes one in a project's own root, which is not a -// discovery unit of its own and is handled by processProject. +// With opts.SlimManifest set, every dbt manifest also gets a slim, +// field-filtered copy (see buildSlimManifest, slimNameFor) written into the +// .astro/ beside it, for the Cosmos Boost plugin to load in place of the full +// manifest at DAG-parse time. That includes any in a project's own root, +// which aren't discovery units of their own and are handled by processProject. func Run(roots []string, version string, opts Options) (Summary, error) { start := time.Now() projectDirs := map[string]bool{} - projectRoots := map[string]string{} // project dir -> root it was found under for _, root := range roots { found, err := findProjects(root) if err != nil { @@ -80,13 +76,12 @@ func Run(roots []string, version string, opts Options) (Summary, error) { } for _, d := range found { projectDirs[d] = true - projectRoots[d] = root } } manifests := map[string]bool{} for _, root := range roots { - found, err := findManifests(root, projectDirs, opts.ManifestNames) + found, err := findManifests(root, projectDirs) if err != nil { return Summary{}, fmt.Errorf("scanning %q for manifests: %w", root, err) } @@ -95,13 +90,13 @@ func Run(roots []string, version string, opts Options) (Summary, error) { } } - type unit struct{ kind, path, root string } + type unit struct{ kind, path string } var units []unit for d := range projectDirs { - units = append(units, unit{kindProject, d, projectRoots[d]}) + units = append(units, unit{kindProject, d}) } for m := range manifests { - units = append(units, unit{kindManifest, m, ""}) + units = append(units, unit{kindManifest, m}) } // Composite sort key: path first, kind as the tiebreaker. NUL sorts below // every other byte, so prefix relationships between paths are preserved. @@ -119,7 +114,7 @@ func Run(roots []string, version string, opts Options) (Summary, error) { defer wg.Done() defer func() { <-sem }() // release the slot if u.kind == kindProject { - results[i] = processProject(u.path, u.root, version, opts) + results[i] = processProject(u.path, version, opts) } else { results[i] = processManifest(u.path, version, opts) } @@ -133,8 +128,7 @@ func Run(roots []string, version string, opts Options) (Summary, error) { // processProject hashes one dbt project directory and writes its sidecar. It reads // dbt_project.yml once (readDbtConfig) and threads the result through hashing and the // templated-packages warning, so the file isn't parsed more than once per project. -// root is the discovery root dir was found under, used to resolve opts.ManifestNames. -func processProject(dir, root, version string, opts Options) Result { +func processProject(dir, version string, opts Options) Result { start := time.Now() cfg := readDbtConfig(dir) hash, files, totalBytes, err := hashProject(dir, cfg) @@ -148,37 +142,38 @@ func processProject(dir, root, version string, opts Options) Result { " in dbt_project.yml hold unresolved Jinja templates; using the dbt default directories for exclusion (the real ones may add cache churn)" } - // A manifest.json (or an opts.ManifestNames override) in the project root - // is not a unit of its own - its .astro/ is this project's - so - // findManifests skips it. Slim it here instead, leaving the project's own - // hash as the anchor. + // A manifest-like file in the project root is not a unit of its own - + // its .astro/ is this project's - so findManifests skips it. Slim every + // one found directly here instead, leaving the project's own hash as the + // anchor; the sidecar's filtered_manifest points at the last one + // processed when more than one exists. var filtered *FilteredManifest if opts.SlimManifest { - name := effectiveManifestName(opts.ManifestNames, root, dir) - if doc, _, isDbt, readErr := readManifestDoc(filepath.Join(dir, name)); readErr == nil && isDbt { - // A failed ambiguity check only skips slimming, not the whole - // project: unlike a writeSlimManifest failure below, it leaves no - // partial artifact, and the project's tree hash below doesn't - // depend on it either way. - note := "" - switch ambiguous, sibErr := hasSiblingDbtManifest(dir, name); { - case sibErr != nil: - note = name + " not slimmed: could not check for another manifest in this directory (" + sibErr.Error() + ")" - case ambiguous: - note = name + " not slimmed: another valid dbt manifest shares this directory" - default: + entries, readErr := os.ReadDir(dir) + if readErr != nil { + note := "could not scan for manifests to slim: " + readErr.Error() + if r.Warning != "" { + note = r.Warning + "; " + note + } + r.Warning = note + } else { + for _, e := range entries { + if e.IsDir() || !isManifestCandidateName(e.Name()) { + continue + } + doc, _, isDbt, readErr := readManifestDoc(filepath.Join(dir, e.Name())) + if readErr != nil || !isDbt { + continue + } // Nothing mutates doc afterward here, unlike processManifest. data, _ := json.Marshal(buildSlimManifest(doc, version)) - if filtered, r.Err = writeSlimManifest(dir, data); r.Err != nil { + f, writeErr := writeSlimManifest(dir, e.Name(), data) + if writeErr != nil { + r.Err = writeErr r.Duration = time.Since(start) return r } - } - if note != "" { - if r.Warning != "" { - note = r.Warning + "; " + note - } - r.Warning = note + filtered = f } } } @@ -188,32 +183,24 @@ func processProject(dir, root, version string, opts Options) Result { return r } -// processManifest hashes one manifest.json and writes a sidecar next to it, -// plus a slim, field-filtered copy of the manifest when opts asks for one (see -// buildSlimManifest). A file that isn't a dbt manifest is skipped (nothing is -// written) so unrelated manifest.json files in the project aren't stamped - -// same as one with a sibling dbt manifest (see hasSiblingDbtManifest). +// processManifest hashes one manifest-like file and writes a sidecar next to +// it, plus a slim, field-filtered copy named after it (see slimNameFor) when +// opts asks for one (see buildSlimManifest). A file that isn't actually a dbt +// manifest is skipped (nothing is written), so an unrelated *.json file whose +// name happens to contain "manifest" isn't stamped. func processManifest(path, version string, opts Options) Result { start := time.Now() doc, bytes, isDbt, err := readManifestDoc(path) var hash string var slimData []byte - var skipNote string if err == nil && isDbt { - switch ambiguous, sibErr := hasSiblingDbtManifest(filepath.Dir(path), filepath.Base(path)); { - case sibErr != nil: - skipNote = "could not check for another manifest in this directory (" + sibErr.Error() + ")" - case ambiguous: - skipNote = "another valid dbt manifest shares this directory; skipped to avoid stamping the wrong one's cache" - default: - if opts.SlimManifest { - // Marshal before hashDocument mutates doc: the slim manifest shares - // doc's nested values, so only turning it into bytes here decouples - // the two. It holds JSON-native types only, so this cannot fail. - slimData, _ = json.Marshal(buildSlimManifest(doc, version)) - } - hash = hashDocument(doc) + if opts.SlimManifest { + // Marshal before hashDocument mutates doc: the slim manifest shares + // doc's nested values, so only turning it into bytes here decouples + // the two. It holds JSON-native types only, so this cannot fail. + slimData, _ = json.Marshal(buildSlimManifest(doc, version)) } + hash = hashDocument(doc) } r := Result{Kind: kindManifest, Path: path, Hash: hash, Files: 1, Bytes: bytes, Duration: time.Since(start)} switch { @@ -221,9 +208,6 @@ func processManifest(path, version string, opts Options) Result { r.Err = err case !isDbt: r.Skipped = true - case skipNote != "": - r.Skipped = true - r.Warning = skipNote default: dir := filepath.Dir(path) // The sidecar goes last: it carries the filtered_manifest pointer, so it @@ -232,7 +216,7 @@ func processManifest(path, version string, opts Options) Result { // safe. var filtered *FilteredManifest if slimData != nil { - filtered, r.Err = writeSlimManifest(dir, slimData) + filtered, r.Err = writeSlimManifest(dir, filepath.Base(path), slimData) } if r.Err == nil { r.Err = writeSidecar(dir, algoManifestJSON, hash, version, filtered) @@ -241,16 +225,18 @@ func processManifest(path, version string, opts Options) Result { return r } -// writeSlimManifest writes data as dir's slim manifest and returns the sidecar -// pointer describing it. data must already be marshaled, so a caller that later -// mutates the source doc cannot leak into it (see processManifest). -func writeSlimManifest(dir string, data []byte) (*FilteredManifest, error) { - if err := writeArtifact(dir, slimManifestName, data); err != nil { +// writeSlimManifest writes data as the slim companion of manifestFilename +// (see slimNameFor) inside dir, and returns the sidecar pointer describing +// it. data must already be marshaled, so a caller that later mutates the +// source doc cannot leak into it (see processManifest). +func writeSlimManifest(dir, manifestFilename string, data []byte) (*FilteredManifest, error) { + name := slimNameFor(manifestFilename) + if err := writeArtifact(dir, name, data); err != nil { return nil, err } return &FilteredManifest{ Schema: slimSchemaVersion, - Path: slimManifestName, + Path: name, Version: ProjectVersion{Algo: algoFilteredManifest, Hash: sha256Hex(data)}, }, nil } @@ -266,8 +252,8 @@ func (s Summary) CountFailed() int { return n } -// CountSkipped returns the number of units skipped: not a dbt manifest, or an -// ambiguous directory (see hasSiblingDbtManifest). +// CountSkipped returns the number of units skipped (manifest-like files that +// aren't dbt manifests). func (s Summary) CountSkipped() int { n := 0 for _, r := range s.Results { @@ -298,8 +284,6 @@ func (s Summary) WriteReport(w io.Writer) { switch { case r.Err != nil: fmt.Fprintf(w, " %s %-8s %s (%v)\n", glyphFail, r.Kind, r.Path, r.Err) - case r.Skipped && r.Warning != "": - fmt.Fprintf(w, " %s %-8s %s (%s)\n", glyphLeft, r.Kind, r.Path, r.Warning) case r.Skipped: fmt.Fprintf(w, " %s %-8s %s (not a dbt manifest)\n", glyphLeft, r.Kind, r.Path) default: diff --git a/pkg/cosmosboost/precompute/precompute_test.go b/pkg/cosmosboost/precompute/precompute_test.go index 36fba714c..06c8666b3 100644 --- a/pkg/cosmosboost/precompute/precompute_test.go +++ b/pkg/cosmosboost/precompute/precompute_test.go @@ -126,105 +126,97 @@ func TestRunSkipsNonDBTManifest(t *testing.T) { } } -// TestRunSkipsAmbiguousDirectoryEvenWithOverride: naming one of two valid dbt -// manifests via ManifestNames must not stamp it - the plugin resolves both -// artifacts by directory alone, so the other manifest's DAG would load them. -func TestRunSkipsAmbiguousDirectoryEvenWithOverride(t *testing.T) { - root := t.TempDir() - writeFiles(t, root, map[string]string{ - "manifests_per_schedule/manifest_global_daily_schedule.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{"model.daily":{"name":"daily"}}}`, - "manifests_per_schedule/manifest_full.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{"model.full":{"name":"full"}}}`, - }) - want := filepath.Join(root, "manifests_per_schedule", "manifest_full.json") - - summary, err := Run([]string{root}, "test", Options{SlimManifest: true, ManifestNames: map[string]string{"manifests_per_schedule": "manifest_full.json"}}) - if err != nil { - t.Fatal(err) - } - if len(summary.Results) != 1 || summary.Results[0].Path != want || !summary.Results[0].Skipped || summary.Results[0].Warning == "" { - t.Fatalf("want 1 skipped result with a reason, got %+v", summary.Results) - } - if _, err := os.Stat(filepath.Join(root, "manifests_per_schedule", sidecarDir)); !os.IsNotExist(err) { - t.Fatalf("an ambiguous directory must get no .astro/ at all: %v", err) +// TestIsManifestCandidateName pins the name-matching rule discovery is built +// on: case-insensitive, "manifest" anywhere in a *.json filename. +func TestIsManifestCandidateName(t *testing.T) { + cases := map[string]bool{ + "manifest.json": true, + "manifest_full.json": true, + "MANIFEST.JSON": true, + "dbt_manifest_v2.json": true, + "run_results.json": false, + "catalog.json": false, + "manifest.txt": false, + "manifest": false, + } + for name, want := range cases { + if got := isManifestCandidateName(name); got != want { + t.Errorf("isManifestCandidateName(%q) = %v, want %v", name, got, want) + } } } -// TestRunDoesNotFlagOtherDbtArtifactsAsAmbiguous: the ordinary case of a -// compiled project - target/manifest.json sitting beside target/run_results.json -// and target/catalog.json, which "dbt build" and "dbt docs generate" always -// produce - must not trip hasSiblingDbtManifest. Regression test for a false -// positive that would have silently disabled stamping for most real projects. -func TestRunDoesNotFlagOtherDbtArtifactsAsAmbiguous(t *testing.T) { +// TestRunSlimsEveryManifestInADirectory: two custom-named manifests sharing a +// directory each get their own slim file, named after themselves - no +// collision, no picking a winner. +func TestRunSlimsEveryManifestInADirectory(t *testing.T) { root := t.TempDir() writeFiles(t, root, map[string]string{ - "proj/dbt_project.yml": "name: shop\n", - "proj/models/a.sql": "select 1", - "proj/target/manifest.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, - "proj/target/run_results.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/run-results/v6.json"}}`, - "proj/target/catalog.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/catalog/v1.json"},"nodes":{}}`, + "manifests_per_schedule/manifest_global_daily_schedule.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{"model.daily":{"name":"daily"}}}`, + "manifests_per_schedule/manifest_full.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{"model.full":{"name":"full"}}}`, }) summary, err := Run([]string{root}, "test", Options{SlimManifest: true}) if err != nil { t.Fatal(err) } - kinds := map[string]int{} + if len(summary.Results) != 2 { + t.Fatalf("want 2 manifest results, got %+v", summary.Results) + } for _, r := range summary.Results { if r.Err != nil || r.Skipped { t.Fatalf("unexpected non-success result: %+v", r) } - kinds[r.Kind]++ } - if kinds["project"] != 1 || kinds["manifest"] != 1 { - t.Fatalf("want 1 project + 1 manifest, got %+v (%+v)", kinds, summary.Results) - } - mustExist(t, filepath.Join(root, "proj", "target", sidecarDir, slimManifestName)) + mustExist(t, filepath.Join(root, "manifests_per_schedule", sidecarDir, "manifest_global_daily_schedule.slim.json")) + mustExist(t, filepath.Join(root, "manifests_per_schedule", sidecarDir, "manifest_full.slim.json")) } -// TestRunManifestNameOverrideExcludesDefaultName: an override replaces -// manifest.json rather than adding to it. -func TestRunManifestNameOverrideExcludesDefaultName(t *testing.T) { +// TestRunDoesNotFlagOtherDbtArtifactsAsManifests: run_results.json and +// catalog.json (which "dbt build"/"dbt docs generate" always produce beside +// manifest.json) don't contain "manifest" in their name; semantic_manifest.json +// does, but its schema URL isn't a manifest one, so content rejects it too. +func TestRunDoesNotFlagOtherDbtArtifactsAsManifests(t *testing.T) { root := t.TempDir() writeFiles(t, root, map[string]string{ - "shipped/manifest.json": `{"name":"My App","icons":[]}`, - "shipped/manifest_full.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, + "proj/dbt_project.yml": "name: shop\n", + "proj/models/a.sql": "select 1", + "proj/target/manifest.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, + "proj/target/run_results.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/run-results/v6.json"}}`, + "proj/target/catalog.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/catalog/v1.json"},"nodes":{}}`, + "proj/target/semantic_manifest.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/semantic-manifest/v1.json"}}`, }) - want := filepath.Join(root, "shipped", "manifest_full.json") - summary, err := Run([]string{root}, "test", Options{ManifestNames: map[string]string{"shipped": "manifest_full.json"}}) + summary, err := Run([]string{root}, "test", Options{SlimManifest: true}) if err != nil { t.Fatal(err) } - if len(summary.Results) != 1 || summary.Results[0].Path != want || summary.Results[0].Err != nil || summary.Results[0].Skipped { - t.Fatalf("want 1 stamped result for manifest_full.json only, manifest.json must be ignored: %+v", summary.Results) + var succeeded, skipped int + for _, r := range summary.Results { + if r.Err != nil { + t.Fatalf("unexpected error: %+v", r) + } + if r.Skipped { + if filepath.Base(r.Path) != "semantic_manifest.json" { + t.Fatalf("unexpected skip: %+v", r) + } + skipped++ + continue + } + succeeded++ } - mustExist(t, filepath.Join(root, "shipped", sidecarDir, sidecarName)) -} - -// TestRunManifestNameOverrideForNestedProject: a dbt project living under -// dags/ (a common Astro layout) is keyed by its full path relative to the -// deploy root, not by its own name alone. -func TestRunManifestNameOverrideForNestedProject(t *testing.T) { - root := t.TempDir() - writeFiles(t, root, map[string]string{ - "dags/dbt/shop/dbt_project.yml": "name: shop\n", - "dags/dbt/shop/models/a.sql": "select 1", - "dags/dbt/shop/manifest_custom.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json","project_name":"shop"},"nodes":{"model.shop.a":{"original_file_path":"models/a.sql","package_name":"shop","resource_type":"model","fqn":["shop","a"]}}}`, - }) - - summary, err := Run([]string{root}, "test", Options{SlimManifest: true, ManifestNames: map[string]string{"dags/dbt/shop": "manifest_custom.json"}}) - if err != nil { - t.Fatal(err) + if succeeded != 2 || skipped != 1 { // 1 project + 1 manifest succeed, semantic_manifest.json is skipped + t.Fatalf("want 2 successes + 1 skip, got %d successes, %d skipped: %+v", succeeded, skipped, summary.Results) } - if len(summary.Results) != 1 || summary.Results[0].Kind != kindProject || summary.Results[0].Err != nil { - t.Fatalf("want 1 project-only result, got %+v", summary.Results) + mustExist(t, filepath.Join(root, "proj", "target", sidecarDir, slimManifestName)) + if _, err := os.Stat(filepath.Join(root, "proj", "target", sidecarDir, "semantic_manifest.slim.json")); !os.IsNotExist(err) { + t.Fatalf("semantic_manifest.json must not be treated as a manifest: %v", err) } - mustExist(t, filepath.Join(root, "dags", "dbt", "shop", sidecarDir, slimManifestName)) } -// TestRunSkipsSlimInProjectRootWhenAmbiguous: a project root with two valid -// dbt manifests still gets its tree-hash sidecar, but isn't slimmed. -func TestRunSkipsSlimInProjectRootWhenAmbiguous(t *testing.T) { +// TestRunSlimsEveryManifestInProjectRoot: a project root with two manifests +// gets both slimmed, plus its own unaffected tree-hash sidecar. +func TestRunSlimsEveryManifestInProjectRoot(t *testing.T) { root := t.TempDir() writeFiles(t, root, map[string]string{ "proj/dbt_project.yml": "name: shop\n", @@ -237,37 +229,38 @@ func TestRunSkipsSlimInProjectRootWhenAmbiguous(t *testing.T) { if err != nil { t.Fatal(err) } - if len(summary.Results) != 1 || summary.Results[0].Kind != kindProject || summary.Results[0].Err != nil || summary.Results[0].Warning == "" { - t.Fatalf("want 1 project result with a warning, got %+v", summary.Results) + if len(summary.Results) != 1 || summary.Results[0].Kind != kindProject || summary.Results[0].Err != nil { + t.Fatalf("want 1 project-only result, got %+v", summary.Results) } mustExist(t, filepath.Join(root, "proj", sidecarDir, sidecarName)) - if _, err := os.Stat(filepath.Join(root, "proj", sidecarDir, slimManifestName)); !os.IsNotExist(err) { - t.Fatalf("an ambiguous project root must not be slimmed: %v", err) - } + mustExist(t, filepath.Join(root, "proj", sidecarDir, slimManifestName)) + mustExist(t, filepath.Join(root, "proj", sidecarDir, "manifest_full.slim.json")) } -// TestRunManifestNameOverridePerDirectory: two projects with different -// manifest-root conventions in one Run each apply their own override. -func TestRunManifestNameOverridePerDirectory(t *testing.T) { +// TestRunHandlesMultipleProjectsWithDifferentManifestNames: three dbt +// projects, each naming its manifest differently, all get discovered and +// slimmed with no configuration. +func TestRunHandlesMultipleProjectsWithDifferentManifestNames(t *testing.T) { root := t.TempDir() writeFiles(t, root, map[string]string{ "dbt1/dbt_project.yml": "name: one\n", - "dbt1/models/a.sql": "select 1", - "dbt1/manifest_custom.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json","project_name":"one"},"nodes":{"model.one.a":{"original_file_path":"models/a.sql","package_name":"one","resource_type":"model","fqn":["one","a"]}}}`, + "dbt1/manifest_custom.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, "dbt2/dbt_project.yml": "name: two\n", - "dbt2/models/b.sql": "select 2", - "dbt2/manifest.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json","project_name":"two"},"nodes":{"model.two.b":{"original_file_path":"models/b.sql","package_name":"two","resource_type":"model","fqn":["two","b"]}}}`, + "dbt2/manifest.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, + "dbt3/dbt_project.yml": "name: three\n", + "dbt3/manifest_by_run.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, }) - summary, err := Run([]string{root}, "test", Options{SlimManifest: true, ManifestNames: map[string]string{"dbt1": "manifest_custom.json"}}) + summary, err := Run([]string{root}, "test", Options{SlimManifest: true}) if err != nil { t.Fatal(err) } - if len(summary.Results) != 2 { - t.Fatalf("want 2 project results, got %+v", summary.Results) + if len(summary.Results) != 3 { + t.Fatalf("want 3 project results, got %+v", summary.Results) } - mustExist(t, filepath.Join(root, "dbt1", sidecarDir, slimManifestName)) // found via the override - mustExist(t, filepath.Join(root, "dbt2", sidecarDir, slimManifestName)) // found via the manifest.json default + mustExist(t, filepath.Join(root, "dbt1", sidecarDir, "manifest_custom.slim.json")) + mustExist(t, filepath.Join(root, "dbt2", sidecarDir, slimManifestName)) + mustExist(t, filepath.Join(root, "dbt3", sidecarDir, "manifest_by_run.slim.json")) } // TestRunWarnsOnTemplatedPackagesPath verifies a project whose packages-install-path diff --git a/pkg/cosmosboost/precompute/slim.go b/pkg/cosmosboost/precompute/slim.go index c202c95ac..e328dad97 100644 --- a/pkg/cosmosboost/precompute/slim.go +++ b/pkg/cosmosboost/precompute/slim.go @@ -1,11 +1,26 @@ package precompute -const slimManifestName = "manifest.slim.json" +import "strings" + +// slimManifestSuffix marks a slim companion file, and lets Cleanup recognize +// one regardless of which manifest it came from. +const slimManifestSuffix = ".slim.json" + +// slimManifestName is the slim companion for the default "manifest.json". +const slimManifestName = "manifest" + slimManifestSuffix // slimSchemaVersion identifies the allowlist that produced a slim manifest, so // a reader that doesn't recognize it can fall back to the full manifest. const slimSchemaVersion = 1 +// slimNameFor returns manifestFilename's slim companion name, e.g. +// "manifest_full.json" -> "manifest_full.slim.json" - so multiple manifests +// in one directory each get their own, discoverable by the same convention +// a consumer already knows its own manifest_path by. +func slimNameFor(manifestFilename string) string { + return strings.TrimSuffix(manifestFilename, ".json") + slimManifestSuffix +} + // slimSections are the only top-level collections Cosmos loads nodes from // (cosmos/dbt/graph.py::_load_nodes_from_manifest_data). var slimSections = []string{"nodes", "sources", "exposures"} diff --git a/pkg/cosmosboost/precompute/slim_test.go b/pkg/cosmosboost/precompute/slim_test.go index 02d9dbe88..decd2d036 100644 --- a/pkg/cosmosboost/precompute/slim_test.go +++ b/pkg/cosmosboost/precompute/slim_test.go @@ -17,6 +17,25 @@ func parseDoc(t *testing.T, raw string) map[string]any { return doc } +// TestSlimNameFor pins the naming convention a consumer needs to compute a +// manifest's slim companion from its own filename alone, with no sidecar +// lookup required. +func TestSlimNameFor(t *testing.T) { + cases := map[string]string{ + "manifest.json": "manifest.slim.json", + "manifest_full.json": "manifest_full.slim.json", + "manifest_by_schedule.json": "manifest_by_schedule.slim.json", + } + for in, want := range cases { + if got := slimNameFor(in); got != want { + t.Errorf("slimNameFor(%q) = %q, want %q", in, got, want) + } + } + if slimNameFor("manifest.json") != slimManifestName { + t.Errorf("slimNameFor(%q) must equal slimManifestName, the default case's constant", "manifest.json") + } +} + // TestBuildSlimManifestKeepsOnlyAllowedResourceFields pins the field allowlist // Cosmos actually reads from a manifest node - both the DbtNode build // (cosmos/dbt/graph.py::_build_dbt_node_from_manifest_resource) and the outlet diff --git a/pkg/cosmosboost/predeploy.go b/pkg/cosmosboost/predeploy.go index d680790b1..1cb237ada 100644 --- a/pkg/cosmosboost/predeploy.go +++ b/pkg/cosmosboost/predeploy.go @@ -2,11 +2,9 @@ package cosmosboost import ( "bytes" - "encoding/json" "fmt" "io" "os" - "path/filepath" "strings" "github.com/astronomer/astro-cli/pkg/cosmosboost/precompute" @@ -30,45 +28,14 @@ func slimManifestEnabled() bool { return value == "" || util.CheckEnvBool(value) } -// manifestNameEnvVar holds a JSON directory->filename map (see -// precompute.Options.ManifestNames). -const manifestNameEnvVar = "ASTRO_COSMOS_BOOST_MANIFEST_NAME" - -// manifestNameOverrides parses manifestNameEnvVar ("" when unset). Every -// value must be a bare filename: findManifests matches by basename, so a -// path there would silently match nothing. -func manifestNameOverrides() (map[string]string, error) { - value := strings.TrimSpace(os.Getenv(manifestNameEnvVar)) - if value == "" { - return nil, nil - } - var overrides map[string]string - if err := json.Unmarshal([]byte(value), &overrides); err != nil { - return nil, fmt.Errorf("%s must be a JSON object of directory to manifest filename: %w", manifestNameEnvVar, err) - } - for dir, name := range overrides { - if filepath.Base(name) != name { - return nil, fmt.Errorf("%s: %q for directory %q must be a bare filename, not a path", manifestNameEnvVar, name, dir) - } - } - return overrides, nil -} - // PreDeploy runs the Cosmos Boost pre-deploy step over path: every dbt project // (dbt_project.yml) gets a .astro/dbt_metadata.json sidecar carrying its // content hash, which the Cosmos Boost plugin uses as a cache version key at // parse time instead of hashing the project tree itself. Every standalone dbt -// manifest.json gets a hash sidecar too, plus a slim, field-filtered copy for +// manifest gets a hash sidecar too, plus a slim, field-filtered copy for // the plugin to load in place of the full manifest at DAG-parse time. func PreDeploy(path string) error { - manifestNames, err := manifestNameOverrides() - if err != nil { - return err - } - opts := precompute.Options{ - SlimManifest: slimManifestEnabled(), - ManifestNames: manifestNames, - } + opts := precompute.Options{SlimManifest: slimManifestEnabled()} summary, err := precompute.Run([]string{path}, version.CurrVersion, opts) if err != nil { return fmt.Errorf("running the Cosmos Boost pre-deploy step: %w", err) diff --git a/pkg/cosmosboost/predeploy_test.go b/pkg/cosmosboost/predeploy_test.go index 473dcbc44..ba439fb33 100644 --- a/pkg/cosmosboost/predeploy_test.go +++ b/pkg/cosmosboost/predeploy_test.go @@ -101,42 +101,17 @@ func TestSlimManifestEnabled(t *testing.T) { } } -// TestPreDeployRespectsManifestNameEnvVar: a differently-named manifest is -// stamped once the env var names it for the deploy root ("."). -func TestPreDeployRespectsManifestNameEnvVar(t *testing.T) { +// TestPreDeployDiscoversCustomNamedManifest: a manifest not literally named +// manifest.json is found and stamped automatically - by name and content, +// with nothing to configure. +func TestPreDeployDiscoversCustomNamedManifest(t *testing.T) { dir := t.TempDir() manifest := `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}` require.NoError(t, os.WriteFile(filepath.Join(dir, "manifest_full.json"), []byte(manifest), 0o644)) - require.NoError(t, PreDeploy(dir)) - _, err := os.Stat(filepath.Join(dir, ".astro")) - require.True(t, os.IsNotExist(err), "a non-manifest.json name is invisible to discovery without the override") - - t.Setenv(manifestNameEnvVar, `{".": "manifest_full.json"}`) require.NoError(t, PreDeploy(dir)) require.FileExists(t, filepath.Join(dir, artifactRelPath)) -} - -// TestManifestNameOverrides: unset is nil; malformed JSON or a non-bare -// filename value is rejected instead of silently matching nothing. -func TestManifestNameOverrides(t *testing.T) { - t.Setenv(manifestNameEnvVar, "") - got, err := manifestNameOverrides() - require.NoError(t, err) - require.Nil(t, got) - - t.Setenv(manifestNameEnvVar, `{".": "manifest_full.json"}`) - got, err = manifestNameOverrides() - require.NoError(t, err) - require.Equal(t, map[string]string{".": "manifest_full.json"}, got) - - t.Setenv(manifestNameEnvVar, "not-json") - _, err = manifestNameOverrides() - require.ErrorContains(t, err, manifestNameEnvVar) - - t.Setenv(manifestNameEnvVar, `{".": "target/manifest_full.json"}`) - _, err = manifestNameOverrides() - require.ErrorContains(t, err, manifestNameEnvVar) + require.FileExists(t, filepath.Join(dir, ".astro", "manifest_full.slim.json")) } func TestPreDeployNoDbtContentIsANoOp(t *testing.T) { From 6d9ad0ddb6fb1ecd951aafd2e1d6c575f088ddb1 Mon Sep 17 00:00:00 2001 From: Pankaj Singh Date: Wed, 7 Oct 2026 20:08:34 +0530 Subject: [PATCH 10/13] fix: scope manifest hash/slim identity per manifest, not per directory dbt_metadata.json's schema bumps to 2: filtered_manifest (a single pointer) becomes manifests, a map keyed by source filename. Each entry carries that manifest's own content hash and its own slim_manifest pointer, so a directory holding more than one manifest no longer loses every sibling's identity to whichever one wrote last - the exact gap kaxil's and tatiana's review comments both raised. Writing is restructured to make that safe: manifest units still compute (hash + slim bytes) concurrently, but are now written in a second pass grouped by directory, so manifests sharing a directory build one complete map together and write their shared sidecar exactly once, from a single writer - not two goroutines racing the same file. Stress-tested at 300 iterations under real parallelism (GOMAXPROCS=8): zero corrupted or lost-update sidecars. Also along the way: - slimNameFor's suffix trim is now case-insensitive, matching isManifestCandidateName (a manifest named MANIFEST.JSON previously got a malformed MANIFEST.JSON.slim.json). - A manifest candidate in a project root that can't even be read now surfaces a warning instead of vanishing silently (was conflated with "not a dbt manifest"). - Trimmed redundant tests/comments across the branch. Co-Authored-By: Claude --- pkg/cosmosboost/precompute/discover.go | 9 +- pkg/cosmosboost/precompute/hash_test.go | 8 +- pkg/cosmosboost/precompute/metadata.go | 47 ++-- pkg/cosmosboost/precompute/precompute.go | 207 ++++++++++++------ pkg/cosmosboost/precompute/precompute_test.go | 127 ++++++++--- pkg/cosmosboost/precompute/slim.go | 10 +- pkg/cosmosboost/precompute/slim_test.go | 1 + pkg/cosmosboost/predeploy_test.go | 2 +- 8 files changed, 268 insertions(+), 143 deletions(-) diff --git a/pkg/cosmosboost/precompute/discover.go b/pkg/cosmosboost/precompute/discover.go index 3a7ef4f93..08abfd5ea 100644 --- a/pkg/cosmosboost/precompute/discover.go +++ b/pkg/cosmosboost/precompute/discover.go @@ -51,12 +51,9 @@ var manifestSkipDirs = map[string]bool{ gitDir: true, // VCS internals can't hold a project's manifest } -// isManifestCandidateName reports whether name could be a dbt manifest, by -// name alone: a *.json file whose name contains "manifest" (case-insensitive) -// - covers manifest.json, manifest_full.json, manifest_by_schedule.json, etc. -// without needing a customer to configure anything. Content is validated -// separately (isDbtManifest) before anything gets stamped, which is what -// keeps this from also matching e.g. semantic_manifest.json. +// isManifestCandidateName reports whether a *.json file could be a dbt +// manifest by name alone (case-insensitive "manifest" anywhere); content is +// validated separately (isDbtManifest). func isManifestCandidateName(name string) bool { lower := strings.ToLower(name) return strings.HasSuffix(lower, ".json") && strings.Contains(lower, "manifest") diff --git a/pkg/cosmosboost/precompute/hash_test.go b/pkg/cosmosboost/precompute/hash_test.go index f3af1c7b9..7947e9714 100644 --- a/pkg/cosmosboost/precompute/hash_test.go +++ b/pkg/cosmosboost/precompute/hash_test.go @@ -220,11 +220,9 @@ func TestHashManifestIgnoresVolatileMetadata(t *testing.T) { } } -// TestHashManifestSkipsNonDBT verifies that a manifest.json lacking the dbt -// shape (e.g. a web-app/PWA manifest), invalid JSON, or another dbt artifact -// entirely (run_results.json, catalog.json - both carry the same -// metadata.dbt_schema_version field dbt manifests do) is not treated as a dbt -// manifest, so it won't be stamped. +// TestHashManifestSkipsNonDBT: a non-dbt manifest.json, invalid JSON, or +// another dbt artifact (same metadata.dbt_schema_version field) isn't +// treated as a dbt manifest. func TestHashManifestSkipsNonDBT(t *testing.T) { dir := t.TempDir() cases := map[string]string{ diff --git a/pkg/cosmosboost/precompute/metadata.go b/pkg/cosmosboost/precompute/metadata.go index f10247451..0b74846bf 100644 --- a/pkg/cosmosboost/precompute/metadata.go +++ b/pkg/cosmosboost/precompute/metadata.go @@ -11,8 +11,9 @@ import ( const application = "astro" const ( - // schemaVersion lets the read-side cope with future format changes. - schemaVersion = 1 + // schemaVersion lets the read-side cope with future format changes. v2: + // filtered_manifest -> manifests, keyed by source filename. + schemaVersion = 2 // algoProjectTree hashes a whole dbt project directory (source files). // v2 excludes .git, which never ships in a deploy payload, so VCS activity @@ -37,25 +38,27 @@ const ( sidecarPerm = 0o644 ) -// Metadata is the content of the .astro/dbt_metadata.json sidecar. The Cosmos -// Boost plugin reads the version.hash field and uses it as the cache version key; -// it never recomputes the hash itself. +// Metadata is the content of the .astro/dbt_metadata.json sidecar. type Metadata struct { Schema int `json:"schema"` Version ProjectVersion `json:"version"` GeneratedBy GeneratedBy `json:"generated_by"` - // FilteredManifest points at the slim manifest beside this sidecar, absent - // when none was written. The sidecar is the consumer's entry point: it - // already reads and schema-gates this file, so it discovers the slim - // manifest here and falls back to the full one when the section is missing. - FilteredManifest *FilteredManifest `json:"filtered_manifest,omitempty"` + // Manifests is keyed by manifest filename, so a consumer looks up its own + // entry instead of trusting Version when a directory has several. + Manifests map[string]ManifestVersion `json:"manifests,omitempty"` } -// FilteredManifest describes a slim manifest sitting next to the sidecar. Path -// is relative to the sidecar's directory, and Version hashes the slim file's -// own bytes, so a consumer can confirm the two are a matched pair without -// re-hashing the full manifest. -type FilteredManifest struct { +// ManifestVersion is one manifest's own hash, plus its slim companion, if +// one was written. +type ManifestVersion struct { + Version ProjectVersion `json:"version"` + Slim *SlimManifest `json:"slim_manifest,omitempty"` +} + +// SlimManifest describes a slim manifest sitting next to the sidecar. Version +// hashes the slim file's own bytes, so a consumer can confirm the two are a +// matched pair without re-hashing the full manifest. +type SlimManifest struct { Schema int `json:"schema"` Path string `json:"path"` Version ProjectVersion `json:"version"` @@ -74,15 +77,13 @@ type GeneratedBy struct { Version string `json:"version"` } -// writeSidecar writes .astro/dbt_metadata.json inside dir. version is the -// producer's version, recorded in generated_by. filtered describes a slim -// manifest written beside it, or is nil when none was. -func writeSidecar(dir, algo, hash, version string, filtered *FilteredManifest) error { +// writeSidecar writes .astro/dbt_metadata.json inside dir. +func writeSidecar(dir, algo, hash, version string, manifests map[string]ManifestVersion) error { meta := Metadata{ - Schema: schemaVersion, - Version: ProjectVersion{Algo: algo, Hash: hash}, - GeneratedBy: GeneratedBy{Application: application, Version: version}, - FilteredManifest: filtered, + Schema: schemaVersion, + Version: ProjectVersion{Algo: algo, Hash: hash}, + GeneratedBy: GeneratedBy{Application: application, Version: version}, + Manifests: manifests, } // Metadata holds only strings and ints, so marshaling cannot fail. diff --git a/pkg/cosmosboost/precompute/precompute.go b/pkg/cosmosboost/precompute/precompute.go index f9207012b..366a3af88 100644 --- a/pkg/cosmosboost/precompute/precompute.go +++ b/pkg/cosmosboost/precompute/precompute.go @@ -19,6 +19,8 @@ const ( kindManifest = "manifest" ) +type unit struct{ kind, path string } + // Result records what happened for one unit of work: either a dbt project // directory or a standalone manifest. type Result struct { @@ -50,21 +52,18 @@ type Options struct { // Run finds every dbt project (a directory with dbt_project.yml) and every // standalone dbt manifest under the given roots, and writes a -// .astro/dbt_metadata.json hash sidecar next to each. Units are processed -// concurrently — one worker each, bounded by GOMAXPROCS — and each is hashed -// over sorted input, so results are deterministic with no cross-worker -// coordination. +// .astro/dbt_metadata.json hash sidecar next to each. version is recorded in +// each sidecar's generated_by. // -// Per-unit failures are best-effort: a unit that fails is recorded in its Result -// and does not stop the others. Run only returns a non-nil error for a top-level -// problem, such as a root that cannot be walked. version is recorded in each -// sidecar's generated_by. +// Per-unit failures are best-effort: a unit that fails is recorded in its +// Result and does not stop the others. Run only returns a non-nil error for a +// top-level problem, such as a root that cannot be walked. // -// With opts.SlimManifest set, every dbt manifest also gets a slim, -// field-filtered copy (see buildSlimManifest, slimNameFor) written into the -// .astro/ beside it, for the Cosmos Boost plugin to load in place of the full -// manifest at DAG-parse time. That includes any in a project's own root, -// which aren't discovery units of their own and are handled by processProject. +// Manifest units are computed concurrently, then written in a second pass +// grouped by directory: two manifests sharing a directory (see +// isManifestCandidateName) share one dbt_metadata.json, so writing it twice +// from two goroutines would race. Project units need no such grouping - each +// owns its directory exclusively. func Run(roots []string, version string, opts Options) (Summary, error) { start := time.Now() @@ -90,7 +89,6 @@ func Run(roots []string, version string, opts Options) (Summary, error) { } } - type unit struct{ kind, path string } var units []unit for d := range projectDirs { units = append(units, unit{kindProject, d}) @@ -105,6 +103,7 @@ func Run(roots []string, version string, opts Options) (Summary, error) { }) results := make([]Result, len(units)) + computations := make([]manifestComputation, len(units)) sem := make(chan struct{}, max(1, runtime.GOMAXPROCS(0))) var wg sync.WaitGroup for i, u := range units { @@ -116,12 +115,22 @@ func Run(roots []string, version string, opts Options) (Summary, error) { if u.kind == kindProject { results[i] = processProject(u.path, version, opts) } else { - results[i] = processManifest(u.path, version, opts) + computations[i] = computeManifest(u.path, version, opts) } }(i, u) } wg.Wait() + byDir := map[string][]int{} + for i, u := range units { + if u.kind == kindManifest { + byDir[filepath.Dir(u.path)] = append(byDir[filepath.Dir(u.path)], i) + } + } + for dir, idxs := range byDir { + writeManifestGroup(dir, idxs, units, computations, results, version) + } + return Summary{Duration: time.Since(start), Results: results}, nil } @@ -145,96 +154,158 @@ func processProject(dir, version string, opts Options) Result { // A manifest-like file in the project root is not a unit of its own - // its .astro/ is this project's - so findManifests skips it. Slim every // one found directly here instead, leaving the project's own hash as the - // anchor; the sidecar's filtered_manifest points at the last one - // processed when more than one exists. - var filtered *FilteredManifest + // anchor. + manifests := map[string]ManifestVersion{} if opts.SlimManifest { entries, readErr := os.ReadDir(dir) if readErr != nil { - note := "could not scan for manifests to slim: " + readErr.Error() - if r.Warning != "" { - note = r.Warning + "; " + note - } - r.Warning = note + r.Warning = joinNotes(r.Warning, "could not scan for manifests to slim: "+readErr.Error()) } else { for _, e := range entries { if e.IsDir() || !isManifestCandidateName(e.Name()) { continue } doc, _, isDbt, readErr := readManifestDoc(filepath.Join(dir, e.Name())) - if readErr != nil || !isDbt { + if readErr != nil { + r.Warning = joinNotes(r.Warning, e.Name()+": could not read as a manifest ("+readErr.Error()+")") + continue + } + if !isDbt { continue } - // Nothing mutates doc afterward here, unlike processManifest. + // Nothing mutates doc afterward here, unlike computeManifest. data, _ := json.Marshal(buildSlimManifest(doc, version)) - f, writeErr := writeSlimManifest(dir, e.Name(), data) + slim, writeErr := writeSlimManifest(dir, e.Name(), data) if writeErr != nil { r.Err = writeErr r.Duration = time.Since(start) return r } - filtered = f + manifests[e.Name()] = ManifestVersion{ + Version: ProjectVersion{Algo: algoManifestJSON, Hash: hashDocument(doc)}, + Slim: slim, + } } } } - r.Err = writeSidecar(dir, algoProjectTree, hash, version, filtered) + r.Err = writeSidecar(dir, algoProjectTree, hash, version, manifests) r.Duration = time.Since(start) return r } -// processManifest hashes one manifest-like file and writes a sidecar next to -// it, plus a slim, field-filtered copy named after it (see slimNameFor) when -// opts asks for one (see buildSlimManifest). A file that isn't actually a dbt -// manifest is skipped (nothing is written), so an unrelated *.json file whose -// name happens to contain "manifest" isn't stamped. -func processManifest(path, version string, opts Options) Result { - start := time.Now() +// manifestComputation is what one manifest unit produces before any writes - +// Run groups these by directory so siblings share one sidecar write instead +// of racing each other for it (see writeManifestGroup). +type manifestComputation struct { + start time.Time + bytes int64 + isDbt bool + hash string + slimData []byte + err error +} + +// computeManifest reads and hashes one manifest-like file, and builds its +// slim copy when opts asks for one. A file that isn't actually a dbt manifest +// is left unhashed, so an unrelated *.json file whose name happens to contain +// "manifest" isn't stamped. +func computeManifest(path, version string, opts Options) manifestComputation { + c := manifestComputation{start: time.Now()} doc, bytes, isDbt, err := readManifestDoc(path) - var hash string - var slimData []byte - if err == nil && isDbt { - if opts.SlimManifest { - // Marshal before hashDocument mutates doc: the slim manifest shares - // doc's nested values, so only turning it into bytes here decouples - // the two. It holds JSON-native types only, so this cannot fail. - slimData, _ = json.Marshal(buildSlimManifest(doc, version)) - } - hash = hashDocument(doc) + c.bytes = bytes + c.isDbt = isDbt + if err != nil { + c.err = err + return c } - r := Result{Kind: kindManifest, Path: path, Hash: hash, Files: 1, Bytes: bytes, Duration: time.Since(start)} - switch { - case err != nil: - r.Err = err - case !isDbt: - r.Skipped = true - default: - dir := filepath.Dir(path) - // The sidecar goes last: it carries the filtered_manifest pointer, so it - // must never exist before the file it points at. Stopping short of it - // looks like "nothing was stamped", which BestEffortPreDeploy treats as - // safe. - var filtered *FilteredManifest - if slimData != nil { - filtered, r.Err = writeSlimManifest(dir, filepath.Base(path), slimData) + if !isDbt { + return c + } + if opts.SlimManifest { + // Marshal before hashDocument mutates doc: the slim manifest shares + // doc's nested values, so only turning it into bytes here decouples + // the two. It holds JSON-native types only, so this cannot fail. + c.slimData, _ = json.Marshal(buildSlimManifest(doc, version)) + } + c.hash = hashDocument(doc) + return c +} + +// writeManifestGroup writes the slim file for each manifest in idxs that +// computed successfully, then one shared sidecar naming all of them - +// idxs are every kindManifest unit found in dir, so this is the directory's +// only writer. Units whose computation failed, or weren't dbt manifests, get +// their Result set here too and are left out of the sidecar. +func writeManifestGroup(dir string, idxs []int, units []unit, computations []manifestComputation, results []Result, version string) { + manifests := map[string]ManifestVersion{} + for _, i := range idxs { + path, c := units[i].path, computations[i] + r := Result{Kind: kindManifest, Path: path, Files: 1, Bytes: c.bytes} + switch { + case c.err != nil: + r.Err = c.err + case !c.isDbt: + r.Skipped = true + default: + r.Hash = c.hash + var slim *SlimManifest + if c.slimData != nil { + slim, r.Err = writeSlimManifest(dir, filepath.Base(path), c.slimData) + } + if r.Err == nil { + manifests[filepath.Base(path)] = ManifestVersion{ + Version: ProjectVersion{Algo: algoManifestJSON, Hash: c.hash}, + Slim: slim, + } + } } - if r.Err == nil { - r.Err = writeSidecar(dir, algoManifestJSON, hash, version, filtered) + r.Duration = time.Since(c.start) + results[i] = r + } + + if len(manifests) == 0 { + return + } + // The sidecar's top-level Version mirrors one manifest for a reader that + // doesn't yet look at Manifests; sorted keys make that choice stable + // across runs rather than whichever unit's goroutine finished last. + names := make([]string, 0, len(manifests)) + for name := range manifests { + names = append(names, name) + } + sort.Strings(names) + primary := manifests[names[0]] + + sidecarErr := writeSidecar(dir, primary.Version.Algo, primary.Version.Hash, version, manifests) + if sidecarErr == nil { + return + } + for _, i := range idxs { + if results[i].Err == nil && !results[i].Skipped { + results[i].Err = sidecarErr } } - return r +} + +// joinNotes appends add to existing, semicolon-separated. +func joinNotes(existing, add string) string { + if existing == "" { + return add + } + return existing + "; " + add } // writeSlimManifest writes data as the slim companion of manifestFilename -// (see slimNameFor) inside dir, and returns the sidecar pointer describing -// it. data must already be marshaled, so a caller that later mutates the -// source doc cannot leak into it (see processManifest). -func writeSlimManifest(dir, manifestFilename string, data []byte) (*FilteredManifest, error) { +// (see slimNameFor) inside dir, and returns the sidecar entry describing it. +// data must already be marshaled, so a caller that later mutates the source +// doc cannot leak into it. +func writeSlimManifest(dir, manifestFilename string, data []byte) (*SlimManifest, error) { name := slimNameFor(manifestFilename) if err := writeArtifact(dir, name, data); err != nil { return nil, err } - return &FilteredManifest{ + return &SlimManifest{ Schema: slimSchemaVersion, Path: name, Version: ProjectVersion{Algo: algoFilteredManifest, Hash: sha256Hex(data)}, diff --git a/pkg/cosmosboost/precompute/precompute_test.go b/pkg/cosmosboost/precompute/precompute_test.go index 06c8666b3..542099e4c 100644 --- a/pkg/cosmosboost/precompute/precompute_test.go +++ b/pkg/cosmosboost/precompute/precompute_test.go @@ -146,30 +146,55 @@ func TestIsManifestCandidateName(t *testing.T) { } } -// TestRunSlimsEveryManifestInADirectory: two custom-named manifests sharing a -// directory each get their own slim file, named after themselves - no -// collision, no picking a winner. -func TestRunSlimsEveryManifestInADirectory(t *testing.T) { +// TestRunSharedDirectorySidecarDescribesBothManifests: two manifests sharing +// a directory outside a project write one shared sidecar, but each keeps its +// own entry (keyed by filename) in Manifests, with its own hash and slim +// pointer - neither is lost to the other. +func TestRunSharedDirectorySidecarDescribesBothManifests(t *testing.T) { root := t.TempDir() writeFiles(t, root, map[string]string{ - "manifests_per_schedule/manifest_global_daily_schedule.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{"model.daily":{"name":"daily"}}}`, - "manifests_per_schedule/manifest_full.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{"model.full":{"name":"full"}}}`, + "shared/manifest_a.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{"model.a":{"name":"a"}}}`, + "shared/manifest_b.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{"model.b":{"name":"b"}}}`, }) summary, err := Run([]string{root}, "test", Options{SlimManifest: true}) if err != nil { t.Fatal(err) } - if len(summary.Results) != 2 { - t.Fatalf("want 2 manifest results, got %+v", summary.Results) - } for _, r := range summary.Results { - if r.Err != nil || r.Skipped { - t.Fatalf("unexpected non-success result: %+v", r) + if r.Err != nil { + t.Fatalf("unexpected error: %+v", r) + } + } + mustExist(t, filepath.Join(root, "shared", sidecarDir, "manifest_a.slim.json")) + mustExist(t, filepath.Join(root, "shared", sidecarDir, "manifest_b.slim.json")) + + var meta Metadata + readJSON(t, filepath.Join(root, "shared", sidecarDir, sidecarName), &meta) + if len(meta.Manifests) != 2 { + t.Fatalf("manifests = %+v, want 2 entries", meta.Manifests) + } + for name, want := range map[string]string{"manifest_a.json": "manifest_a.slim.json", "manifest_b.json": "manifest_b.slim.json"} { + entry, ok := meta.Manifests[name] + if !ok || entry.Version.Hash == "" || entry.Slim == nil || entry.Slim.Path != want { + t.Fatalf("Manifests[%q] = %+v, want a hash and slim path %q", name, entry, want) + } + } + if meta.Manifests["manifest_a.json"].Version.Hash == meta.Manifests["manifest_b.json"].Version.Hash { + t.Fatal("the two manifests' own hashes must differ (their content does)") + } + // The top-level Version is for a reader that doesn't yet look at + // Manifests; it should consistently pick the alphabetically-first + // manifest, not whichever goroutine happened to finish last. + for i := 0; i < 20; i++ { + if _, err := Run([]string{root}, "test", Options{SlimManifest: true}); err != nil { + t.Fatal(err) + } + readJSON(t, filepath.Join(root, "shared", sidecarDir, sidecarName), &meta) + if meta.Version.Hash != meta.Manifests["manifest_a.json"].Version.Hash { + t.Fatalf("top-level Version = %+v, want manifest_a.json's (alphabetically first)", meta.Version) } } - mustExist(t, filepath.Join(root, "manifests_per_schedule", sidecarDir, "manifest_global_daily_schedule.slim.json")) - mustExist(t, filepath.Join(root, "manifests_per_schedule", sidecarDir, "manifest_full.slim.json")) } // TestRunDoesNotFlagOtherDbtArtifactsAsManifests: run_results.json and @@ -237,6 +262,39 @@ func TestRunSlimsEveryManifestInProjectRoot(t *testing.T) { mustExist(t, filepath.Join(root, "proj", sidecarDir, "manifest_full.slim.json")) } +// TestRunWarnsOnUnreadableProjectRootManifest: a candidate in the project +// root that can't even be read (unlike one that reads fine but isn't a dbt +// manifest) must surface a warning, not vanish silently - the project still +// gets stamped on its own tree hash either way. +func TestRunWarnsOnUnreadableProjectRootManifest(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("file permission semantics differ on windows") + } + root := t.TempDir() + writeFiles(t, root, map[string]string{ + "proj/dbt_project.yml": "name: shop\n", + "proj/models/a.sql": "select 1", + "proj/manifest_broken.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, + }) + broken := filepath.Join(root, "proj", "manifest_broken.json") + if err := os.Chmod(broken, 0o000); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(broken, 0o644) }) + + summary, err := Run([]string{root}, "test", Options{SlimManifest: true}) + if err != nil { + t.Fatal(err) + } + if len(summary.Results) != 1 || summary.Results[0].Err != nil { + t.Fatalf("want 1 successful project result, got %+v", summary.Results) + } + if summary.Results[0].Warning == "" { + t.Fatal("want a warning naming the unreadable manifest, got none") + } + mustExist(t, filepath.Join(root, "proj", sidecarDir, sidecarName)) +} + // TestRunHandlesMultipleProjectsWithDifferentManifestNames: three dbt // projects, each naming its manifest differently, all get discovered and // slimmed with no configuration. @@ -246,21 +304,18 @@ func TestRunHandlesMultipleProjectsWithDifferentManifestNames(t *testing.T) { "dbt1/dbt_project.yml": "name: one\n", "dbt1/manifest_custom.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, "dbt2/dbt_project.yml": "name: two\n", - "dbt2/manifest.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, - "dbt3/dbt_project.yml": "name: three\n", - "dbt3/manifest_by_run.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, + "dbt2/manifest_by_run.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, }) summary, err := Run([]string{root}, "test", Options{SlimManifest: true}) if err != nil { t.Fatal(err) } - if len(summary.Results) != 3 { - t.Fatalf("want 3 project results, got %+v", summary.Results) + if len(summary.Results) != 2 { + t.Fatalf("want 2 project results, got %+v", summary.Results) } mustExist(t, filepath.Join(root, "dbt1", sidecarDir, "manifest_custom.slim.json")) - mustExist(t, filepath.Join(root, "dbt2", sidecarDir, slimManifestName)) - mustExist(t, filepath.Join(root, "dbt3", sidecarDir, "manifest_by_run.slim.json")) + mustExist(t, filepath.Join(root, "dbt2", sidecarDir, "manifest_by_run.slim.json")) } // TestRunWarnsOnTemplatedPackagesPath verifies a project whose packages-install-path @@ -594,10 +649,8 @@ func TestRunReportsFailedUnits(t *testing.T) { // TestRunWritesSlimManifestAlongsideSidecar: with the option set, every // discovered manifest.json gets a slim copy next to its hash sidecar, and the -// sidecar carries the filtered_manifest pointer at it. The pointer hashes the -// slim file's own bytes, so a consumer can confirm the two are a matched pair - -// which adjacency alone no longer implies, now that cleanup judges each -// artifact independently. +// sidecar's Manifests entry for it carries a pointer hashing the slim file's +// own bytes, so a consumer can confirm the two are a matched pair. func TestRunWritesSlimManifestAlongsideSidecar(t *testing.T) { root := t.TempDir() writeFiles(t, root, map[string]string{ @@ -621,22 +674,23 @@ func TestRunWritesSlimManifestAlongsideSidecar(t *testing.T) { var meta Metadata readJSON(t, filepath.Join(astroDir, sidecarName), &meta) - fm := meta.FilteredManifest - if fm == nil { - t.Fatalf("sidecar has no filtered_manifest section: %+v", meta) + entry, ok := meta.Manifests["manifest.json"] + fm := entry.Slim + if !ok || fm == nil { + t.Fatalf("sidecar has no slim_manifest for manifest.json: %+v", meta) } if fm.Path != slimManifestName || fm.Schema != slimSchemaVersion || fm.Version.Algo != algoFilteredManifest { - t.Fatalf("filtered_manifest = %+v", fm) + t.Fatalf("slim_manifest = %+v", fm) } slimData, err := os.ReadFile(filepath.Join(astroDir, slimManifestName)) if err != nil { t.Fatal(err) } if want := sha256Hex(slimData); fm.Version.Hash != want { - t.Fatalf("filtered_manifest hash = %q, want the slim file's own hash %q", fm.Version.Hash, want) + t.Fatalf("slim_manifest hash = %q, want the slim file's own hash %q", fm.Version.Hash, want) } if fm.Version.Hash == meta.Version.Hash { - t.Fatal("filtered_manifest hash must not be the full manifest's hash") + t.Fatal("slim_manifest hash must not be the full manifest's hash") } } @@ -661,8 +715,8 @@ func TestRunSkipsSlimManifestWhenNotRequested(t *testing.T) { if err != nil { t.Fatal(err) } - if strings.Contains(string(raw), "filtered_manifest") { - t.Fatalf("filtered_manifest emitted with the slim manifest disabled: %s", raw) + if strings.Contains(string(raw), "slim_manifest") { + t.Fatalf("slim_manifest emitted with the slim manifest disabled: %s", raw) } } @@ -700,8 +754,9 @@ func TestRunSlimsManifestInProjectRoot(t *testing.T) { if meta.Version.Algo != algoProjectTree { t.Fatalf("project sidecar algo = %q, want %q", meta.Version.Algo, algoProjectTree) } - if meta.FilteredManifest == nil || meta.FilteredManifest.Path != slimManifestName { - t.Fatalf("project sidecar does not point at the slim manifest: %+v", meta.FilteredManifest) + entry, ok := meta.Manifests["manifest.json"] + if !ok || entry.Slim == nil || entry.Slim.Path != slimManifestName { + t.Fatalf("project sidecar does not point at the slim manifest: %+v", meta.Manifests) } } @@ -749,8 +804,8 @@ func TestRunSkipsNonDbtManifestInProjectRoot(t *testing.T) { } var meta Metadata readJSON(t, filepath.Join(astroDir, sidecarName), &meta) - if meta.FilteredManifest != nil { - t.Fatalf("project sidecar points at a slim manifest that was never written: %+v", meta.FilteredManifest) + if len(meta.Manifests) != 0 { + t.Fatalf("project sidecar lists a manifest that was never slimmed: %+v", meta.Manifests) } } diff --git a/pkg/cosmosboost/precompute/slim.go b/pkg/cosmosboost/precompute/slim.go index e328dad97..fb8201165 100644 --- a/pkg/cosmosboost/precompute/slim.go +++ b/pkg/cosmosboost/precompute/slim.go @@ -14,11 +14,13 @@ const slimManifestName = "manifest" + slimManifestSuffix const slimSchemaVersion = 1 // slimNameFor returns manifestFilename's slim companion name, e.g. -// "manifest_full.json" -> "manifest_full.slim.json" - so multiple manifests -// in one directory each get their own, discoverable by the same convention -// a consumer already knows its own manifest_path by. +// "manifest_full.json" -> "manifest_full.slim.json". The trim is +// case-insensitive, matching isManifestCandidateName. func slimNameFor(manifestFilename string) string { - return strings.TrimSuffix(manifestFilename, ".json") + slimManifestSuffix + if len(manifestFilename) >= 5 && strings.EqualFold(manifestFilename[len(manifestFilename)-5:], ".json") { + manifestFilename = manifestFilename[:len(manifestFilename)-5] + } + return manifestFilename + slimManifestSuffix } // slimSections are the only top-level collections Cosmos loads nodes from diff --git a/pkg/cosmosboost/precompute/slim_test.go b/pkg/cosmosboost/precompute/slim_test.go index decd2d036..b60c0cd4b 100644 --- a/pkg/cosmosboost/precompute/slim_test.go +++ b/pkg/cosmosboost/precompute/slim_test.go @@ -25,6 +25,7 @@ func TestSlimNameFor(t *testing.T) { "manifest.json": "manifest.slim.json", "manifest_full.json": "manifest_full.slim.json", "manifest_by_schedule.json": "manifest_by_schedule.slim.json", + "MANIFEST.JSON": "MANIFEST.slim.json", // matches isManifestCandidateName's case-insensitivity } for in, want := range cases { if got := slimNameFor(in); got != want { diff --git a/pkg/cosmosboost/predeploy_test.go b/pkg/cosmosboost/predeploy_test.go index ba439fb33..0385c3efd 100644 --- a/pkg/cosmosboost/predeploy_test.go +++ b/pkg/cosmosboost/predeploy_test.go @@ -45,7 +45,7 @@ func TestPreDeployWritesArtifact(t *testing.T) { } `json:"generated_by"` } require.NoError(t, json.Unmarshal(data, &meta)) - require.Equal(t, 1, meta.Schema, "schema is the plugin's compatibility gate and must stay 1") + require.Equal(t, 2, meta.Schema, "schema is the plugin's compatibility gate - bump deliberately, not by accident") require.NotEmpty(t, meta.Version.Hash, "version.hash is what the plugin consumes") require.NotEmpty(t, meta.Version.Algo) require.Equal(t, "astro", meta.GeneratedBy.Application) From 51ad9937d46b84a9c8e1bcc050c079d5903b800f Mon Sep 17 00:00:00 2001 From: Pankaj Singh Date: Wed, 7 Oct 2026 20:18:09 +0530 Subject: [PATCH 11/13] fix: don't abort project sidecar write on one manifest's slim failure processProject aborted the whole unit (discarding the project's tree hash and any sibling manifest's already-written slim file) if any one candidate's slim write failed. Isolate the failure to that candidate, matching the existing read-error handling in the same loop. Co-Authored-By: Claude --- pkg/cosmosboost/precompute/precompute.go | 8 ++-- pkg/cosmosboost/precompute/precompute_test.go | 38 +++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/pkg/cosmosboost/precompute/precompute.go b/pkg/cosmosboost/precompute/precompute.go index 366a3af88..d91449b98 100644 --- a/pkg/cosmosboost/precompute/precompute.go +++ b/pkg/cosmosboost/precompute/precompute.go @@ -177,9 +177,11 @@ func processProject(dir, version string, opts Options) Result { data, _ := json.Marshal(buildSlimManifest(doc, version)) slim, writeErr := writeSlimManifest(dir, e.Name(), data) if writeErr != nil { - r.Err = writeErr - r.Duration = time.Since(start) - return r + // A failure slimming one candidate doesn't cost the + // project its tree-hash sidecar, or a sibling candidate + // its own already-written slim file. + r.Warning = joinNotes(r.Warning, e.Name()+": could not write its slim manifest ("+writeErr.Error()+")") + continue } manifests[e.Name()] = ManifestVersion{ Version: ProjectVersion{Algo: algoManifestJSON, Hash: hashDocument(doc)}, diff --git a/pkg/cosmosboost/precompute/precompute_test.go b/pkg/cosmosboost/precompute/precompute_test.go index 542099e4c..702c38034 100644 --- a/pkg/cosmosboost/precompute/precompute_test.go +++ b/pkg/cosmosboost/precompute/precompute_test.go @@ -262,6 +262,44 @@ func TestRunSlimsEveryManifestInProjectRoot(t *testing.T) { mustExist(t, filepath.Join(root, "proj", sidecarDir, "manifest_full.slim.json")) } +// TestRunProjectRootSlimFailureIsolatedToOneManifest: one candidate's slim +// write failing (its target path is pre-occupied by a directory, so this +// works even run as root) must not cost the project its tree-hash sidecar, +// or an earlier sibling its already-written slim file. +func TestRunProjectRootSlimFailureIsolatedToOneManifest(t *testing.T) { + root := t.TempDir() + writeFiles(t, root, map[string]string{ + "proj/dbt_project.yml": "name: shop\n", + "proj/models/a.sql": "select 1", + // Alphabetically first, so it's slimmed before the failing one. + "proj/manifest_a.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, + "proj/manifest_b.json": `{"metadata":{"dbt_schema_version":"https://schemas.getdbt.com/dbt/manifest/v12.json"},"nodes":{}}`, + }) + // manifest_b's slim write will fail: its target path is already a directory. + if err := os.MkdirAll(filepath.Join(root, "proj", sidecarDir, "manifest_b.slim.json"), 0o755); err != nil { + t.Fatal(err) + } + + summary, err := Run([]string{root}, "test", Options{SlimManifest: true}) + if err != nil { + t.Fatal(err) + } + if len(summary.Results) != 1 || summary.Results[0].Err != nil || summary.Results[0].Warning == "" { + t.Fatalf("want 1 successful project result with a warning, got %+v", summary.Results) + } + mustExist(t, filepath.Join(root, "proj", sidecarDir, sidecarName)) + mustExist(t, filepath.Join(root, "proj", sidecarDir, "manifest_a.slim.json")) + + var meta Metadata + readJSON(t, filepath.Join(root, "proj", sidecarDir, sidecarName), &meta) + if _, ok := meta.Manifests["manifest_a.json"]; !ok { + t.Fatalf("manifest_a.json missing from sidecar despite slimming successfully: %+v", meta.Manifests) + } + if _, ok := meta.Manifests["manifest_b.json"]; ok { + t.Fatalf("manifest_b.json should not be listed - its slim write failed: %+v", meta.Manifests) + } +} + // TestRunWarnsOnUnreadableProjectRootManifest: a candidate in the project // root that can't even be read (unlike one that reads fine but isn't a dbt // manifest) must surface a warning, not vanish silently - the project still From 217675756203215788c78ba5ed3cca496e330f50 Mon Sep 17 00:00:00 2001 From: Pankaj Singh Date: Wed, 7 Oct 2026 20:25:58 +0530 Subject: [PATCH 12/13] fix: don't fail the whole project hash on an unreadable manifest candidate hashProject's tree walk aborted the entire project (CI's real permission enforcement, unlike this sandbox's root user) when a root-level manifest-candidate file couldn't be read. processProject already re-reads and warns on that same file - skip it here instead of failing the walk. Co-Authored-By: Claude --- pkg/cosmosboost/precompute/hash.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/cosmosboost/precompute/hash.go b/pkg/cosmosboost/precompute/hash.go index f4e6fd622..7331f0cf3 100644 --- a/pkg/cosmosboost/precompute/hash.go +++ b/pkg/cosmosboost/precompute/hash.go @@ -123,6 +123,9 @@ func hashProject(dir string, cfg dbtConfig) (hash string, files int, totalBytes digest, n, err := hashFile(path) if err != nil { + if !strings.Contains(rel, "/") && isManifestCandidateName(d.Name()) { + return nil // processProject re-reads and warns on this one itself + } return err } entries = append(entries, entry{rel, digest}) From 4e999d2dbeb97749f87e30ae5e8195e9809d80d5 Mon Sep 17 00:00:00 2001 From: Pankaj Singh Date: Thu, 8 Oct 2026 13:57:19 +0530 Subject: [PATCH 13/13] fix: correct stale comment and broaden scaffold gitignore for slim manifests - precompute.go: fix comment claiming doc is never mutated after the slim marshal - hashDocument mutates it right below; the marshal just has to happen first. - scaffold gitignore matched only the default manifest.slim.json, so a custom-named slim file (e.g. manifest_full.slim.json) showed up untracked. Same issue as #2242, broadened the same way. Co-Authored-By: Claude --- pkg/airflowrt/include/gitignore | 2 +- pkg/cosmosboost/precompute/precompute.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/airflowrt/include/gitignore b/pkg/airflowrt/include/gitignore index a549900b8..64c686f31 100644 --- a/pkg/airflowrt/include/gitignore +++ b/pkg/airflowrt/include/gitignore @@ -15,4 +15,4 @@ airflow.db .astro/*.local.yaml **/.astro/dbt_metadata.json -**/.astro/manifest.slim.json +**/.astro/*manifest*.slim.json diff --git a/pkg/cosmosboost/precompute/precompute.go b/pkg/cosmosboost/precompute/precompute.go index d91449b98..10b5796d8 100644 --- a/pkg/cosmosboost/precompute/precompute.go +++ b/pkg/cosmosboost/precompute/precompute.go @@ -173,7 +173,7 @@ func processProject(dir, version string, opts Options) Result { if !isDbt { continue } - // Nothing mutates doc afterward here, unlike computeManifest. + // Marshal before hashDocument mutates doc below (see computeManifest). data, _ := json.Marshal(buildSlimManifest(doc, version)) slim, writeErr := writeSlimManifest(dir, e.Name(), data) if writeErr != nil {