From 2411c549187fab14be39edb765b61b603f780d2b Mon Sep 17 00:00:00 2001 From: "ben.hansen" Date: Wed, 9 Sep 2026 16:13:39 +0000 Subject: [PATCH 1/3] Scope git-pinned AIR snapshots to root path --- .../experimental/air/config-help/output.txt | 8 +++ .../experimental/air/config-help/script | 3 ++ experimental/air/cmd/runconfig.go | 2 +- experimental/air/cmd/snapshot_cachekey.go | 10 ++-- .../air/cmd/snapshot_cachekey_test.go | 24 ++++++--- experimental/air/cmd/snapshot_dabs.go | 35 ++++++------- experimental/air/cmd/snapshot_git.go | 20 ++++++++ experimental/air/cmd/snapshot_git_test.go | 20 ++++++++ experimental/air/cmd/snapshot_package.go | 21 ++++++-- experimental/air/cmd/snapshot_package_test.go | 49 ++++++++++++++++++- experimental/air/cmd/snapshot_resolve.go | 20 +++++--- experimental/air/cmd/snapshot_resolve_test.go | 14 ++++++ 12 files changed, 185 insertions(+), 41 deletions(-) diff --git a/acceptance/experimental/air/config-help/output.txt b/acceptance/experimental/air/config-help/output.txt index e9a066be055..a664084f07e 100644 --- a/acceptance/experimental/air/config-help/output.txt +++ b/acceptance/experimental/air/config-help/output.txt @@ -123,6 +123,14 @@ config.code_source.snapshot.git.remote Type: bool or string Required: no +=== git-pinned subdirectory snapshots document root_path scoping +>>> [CLI] experimental air run -h config.code_source.snapshot.root_path +config.code_source.snapshot.root_path + Root of the code source to archive. A git-pinned subdirectory packages only that subtree. + + Type: string + Required: when code_source.snapshot is set + === the config. prefix is optional >>> [CLI] experimental air run -h compute.num_accelerators config.compute.num_accelerators diff --git a/acceptance/experimental/air/config-help/script b/acceptance/experimental/air/config-help/script index cc16cc38907..31c45547c35 100644 --- a/acceptance/experimental/air/config-help/script +++ b/acceptance/experimental/air/config-help/script @@ -27,6 +27,9 @@ trace $CLI experimental air run -h config.environment.docker_image.url title "union field reports both accepted shapes" trace $CLI experimental air run -h config.code_source.snapshot.git.remote +title "git-pinned subdirectory snapshots document root_path scoping" +trace $CLI experimental air run -h config.code_source.snapshot.root_path + title "the config. prefix is optional" trace $CLI experimental air run -h compute.num_accelerators diff --git a/experimental/air/cmd/runconfig.go b/experimental/air/cmd/runconfig.go index 6ba95df3fa3..5fe5699d238 100644 --- a/experimental/air/cmd/runconfig.go +++ b/experimental/air/cmd/runconfig.go @@ -384,7 +384,7 @@ func (c *codeSourceConfig) validate() error { // snapshotSourceConfig describes a local directory to tar and upload. type snapshotSourceConfig struct { - RootPath string `yaml:"root_path" help:"Local directory to archive, relative or absolute." required:"when code_source.snapshot is set"` + RootPath string `yaml:"root_path" help:"Root of the code source to archive. A git-pinned subdirectory packages only that subtree." required:"when code_source.snapshot is set"` RemoteVolume *string `yaml:"remote_volume" help:"Volume to upload the archive to. Must start with /Volumes/."` Git *gitRef `yaml:"git" help:"Pin the snapshot to a specific git revision."` IncludePaths []string `yaml:"include_paths" help:"Restrict the archive to these paths, relative to root_path and without \"..\". Omit to include everything."` diff --git a/experimental/air/cmd/snapshot_cachekey.go b/experimental/air/cmd/snapshot_cachekey.go index 44c58ee903b..44c5cb8f351 100644 --- a/experimental/air/cmd/snapshot_cachekey.go +++ b/experimental/air/cmd/snapshot_cachekey.go @@ -15,9 +15,10 @@ import ( const snapshotPackagingVersion = "v1" // computeSnapshotCacheKey returns a stable cache key for a snapshot tarball: the -// SHA-256 digest of (commitSHA, normalized includePaths, snapshotPackagingVersion). -// Changing any input yields a different entry. -func computeSnapshotCacheKey(commitSHA string, includePaths []string) string { +// SHA-256 digest of (commitSHA, normalized includePaths, snapshotPackagingVersion, +// subtreePrefix). Changing any input yields a different entry. An empty subtree +// prefix keeps repository-root keys byte-identical to prior versions. +func computeSnapshotCacheKey(commitSHA string, includePaths []string, subtreePrefix string) string { var normalizedPaths string if len(includePaths) > 0 { trimmed := make([]string, len(includePaths)) @@ -29,6 +30,9 @@ func computeSnapshotCacheKey(commitSHA string, includePaths []string) string { } keyMaterial := commitSHA + "\n" + normalizedPaths + "\n" + snapshotPackagingVersion + if subtreePrefix != "" { + keyMaterial += "\n" + subtreePrefix + } sum := sha256.Sum256([]byte(keyMaterial)) return hex.EncodeToString(sum[:]) } diff --git a/experimental/air/cmd/snapshot_cachekey_test.go b/experimental/air/cmd/snapshot_cachekey_test.go index 5743217c003..2f933aa40bd 100644 --- a/experimental/air/cmd/snapshot_cachekey_test.go +++ b/experimental/air/cmd/snapshot_cachekey_test.go @@ -29,7 +29,7 @@ func TestComputeSnapshotCacheKeyGolden(t *testing.T) { for _, tc := range cases { t.Run(tc.Name, func(t *testing.T) { - assert.Equal(t, tc.CacheKey, computeSnapshotCacheKey(tc.CommitSHA, tc.IncludePaths)) + assert.Equal(t, tc.CacheKey, computeSnapshotCacheKey(tc.CommitSHA, tc.IncludePaths, "")) }) } } @@ -41,21 +41,31 @@ func TestComputeSnapshotCacheKeyProperties(t *testing.T) { // Order-independent: sorting means unsorted input yields the sorted key. assert.Equal(t, - computeSnapshotCacheKey(sha, []string{"a", "b", "c"}), - computeSnapshotCacheKey(sha, []string{"c", "a", "b"}), + computeSnapshotCacheKey(sha, []string{"a", "b", "c"}, ""), + computeSnapshotCacheKey(sha, []string{"c", "a", "b"}, ""), ) // nil and empty include_paths are equivalent (both contribute an empty line). - assert.Equal(t, computeSnapshotCacheKey(sha, nil), computeSnapshotCacheKey(sha, []string{})) + assert.Equal(t, computeSnapshotCacheKey(sha, nil, ""), computeSnapshotCacheKey(sha, []string{}, "")) // Paths are trimmed before hashing. assert.Equal(t, - computeSnapshotCacheKey(sha, []string{"research", "data"}), - computeSnapshotCacheKey(sha, []string{" research ", " data "}), + computeSnapshotCacheKey(sha, []string{"research", "data"}, ""), + computeSnapshotCacheKey(sha, []string{" research ", " data "}, ""), ) // Duplicates are NOT collapsed — they are sorted and kept, matching Python. - assert.NotEqual(t, computeSnapshotCacheKey(sha, []string{"x", "y"}), computeSnapshotCacheKey(sha, []string{"x", "x", "y"})) + assert.NotEqual(t, computeSnapshotCacheKey(sha, []string{"x", "y"}, ""), computeSnapshotCacheKey(sha, []string{"x", "x", "y"}, "")) + + // A subtree gets a distinct key while an empty prefix preserves repository-root + // keys. Different subtrees at one commit cannot collide, even if their root + // directories have the same base name. + rootKey := computeSnapshotCacheKey(sha, nil, "") + assert.NotEqual(t, rootKey, computeSnapshotCacheKey(sha, nil, "team_a/src")) + assert.NotEqual(t, + computeSnapshotCacheKey(sha, nil, "team_a/src"), + computeSnapshotCacheKey(sha, nil, "team_b/src"), + ) // The version constant participates: a different version is a different key. assert.NotEqual(t, snapshotPackagingVersion, "") diff --git a/experimental/air/cmd/snapshot_dabs.go b/experimental/air/cmd/snapshot_dabs.go index ed809b09e01..a7e6bccc1e9 100644 --- a/experimental/air/cmd/snapshot_dabs.go +++ b/experimental/air/cmd/snapshot_dabs.go @@ -68,11 +68,12 @@ func snapshotViaDABsUpload(ctx context.Context, w *databricks.WorkspaceClient, s // than failing an otherwise-valid submission. // // The sidecars are deliberately NOT bundled into the code tarball. The git_archive - // tarball is content-addressed and cached by (commit, include_paths), so a second - // run at the same commit reuses it; but the sidecars vary per run (git_state's - // timestamp, and git_diff captures the working tree at submit time). Folding them - // in would force a distinct tarball per run (defeating the cache) or serve a prior - // run's stale provenance on a cache hit. They also live in the per-run launch dir, + // tarball is content-addressed and cached by (commit, include_paths, root_path + // subtree), so a second identical snapshot reuses it; but the sidecars vary per + // run (git_state's timestamp, and git_diff captures the working tree at submit + // time). Folding them in would force a distinct tarball per run (defeating the + // cache) or serve a prior run's stale provenance on a cache hit. They also live in + // the per-run launch dir, // not the shared artifact dir, so they don't accumulate. Keep them out of the tar. if uploadProvenanceSidecars && plan.isGitRepo { result.GitStatePath, result.GitDiffPath = uploadSnapshotSidecars(ctx, sidecarStore, sidecarBase, newGitRepo(repoPath), plan) @@ -127,14 +128,14 @@ func uploadSnapshotSidecars(ctx context.Context, sidecarStore filer.Filer, sidec } // snapshotTarballName is the uploaded filename for the snapshot. It is deterministic -// for git_archive — _.tar.gz keyed on (commit, include_paths) — so -// an identical commit reuses the same remote object (see the cache check below). For -// plain_tar it is timestamped so concurrent submissions of the same directory don't -// clobber each other's upload (working-tree content isn't pinned to a SHA, so it -// can't be content-addressed). +// for git_archive — _.tar.gz keyed on (commit, include_paths, +// root_path subtree) — so an identical snapshot reuses the same remote object (see +// the cache check below). For plain_tar it is timestamped so concurrent submissions +// of the same directory don't clobber each other's upload (working-tree content +// isn't pinned to a SHA, so it can't be content-addressed). func snapshotTarballName(plan snapshotPlan, dirName string) string { if plan.mode == modeGitArchive { - key := computeSnapshotCacheKey(plan.commitSHA, plan.includePaths) + key := computeSnapshotCacheKey(plan.commitSHA, plan.includePaths, plan.subtreePrefix) return fmt.Sprintf("%s_%s.tar.gz", dirName, key[:16]) } return fmt.Sprintf("%s_%s.tar.gz", dirName, time.Now().UTC().Format("20060102_150405")) @@ -145,7 +146,7 @@ func snapshotTarballName(plan snapshotPlan, dirName string) string { func packageSnapshot(ctx context.Context, repoPath string, plan snapshotPlan, tarball string) error { dirName := filepath.Base(repoPath) if plan.mode == modeGitArchive { - return createGitArchiveSnapshot(ctx, newGitRepo(repoPath), plan.commitSHA, tarball, dirName, plan.includePaths) + return createGitArchiveSnapshot(ctx, newGitRepo(repoPath), plan.commitSHA, tarball, dirName, plan.includePaths, plan.subtreePrefix) } return createPlainTarball(ctx, repoPath, tarball, plan.includePaths, plan.isGitRepo) } @@ -157,8 +158,8 @@ func packageSnapshot(ctx context.Context, repoPath string, plan snapshotPlan, ta // tarball goes to that UC Volume; otherwise to the user's repo_snapshots dir. // // git_archive snapshots are cacheable: the tarball name is content-addressed by -// (commit, include_paths), so if the identical object is already uploaded we skip -// packaging and upload entirely and just reuse the remote path. +// (commit, include_paths, root_path subtree), so if the identical object is already +// uploaded we skip packaging and upload entirely and just reuse the remote path. func uploadSnapshotViaDABs(ctx context.Context, w *databricks.WorkspaceClient, repoPath string, plan snapshotPlan, remoteVolume string) (snapshotResult, error) { // artifactPath is where DABs uploads the tarball; GetFilerForLibraries routes to // a Workspace or Volume filer based on its prefix, then appends /.internal. @@ -210,9 +211,9 @@ func uploadSnapshotViaDABs(ctx context.Context, w *databricks.WorkspaceClient, r return snapshotResult{}, err } - // git_archive is cacheable by (commit, include_paths): if the identical tarball is - // already uploaded, skip packaging + upload and reuse it. Only the config-path - // rewrite (ReplaceWithRemotePath) runs — no bytes move. + // git_archive is cacheable by (commit, include_paths, root_path subtree): if the + // identical tarball is already uploaded, skip packaging + upload and reuse it. + // Only the config-path rewrite (ReplaceWithRemotePath) runs — no bytes move. if plan.mode == modeGitArchive { f, uploadPath, diags := libraries.GetFilerForLibraries(ctx, b) if diags.HasError() { diff --git a/experimental/air/cmd/snapshot_git.go b/experimental/air/cmd/snapshot_git.go index 20783664885..4df4700111b 100644 --- a/experimental/air/cmd/snapshot_git.go +++ b/experimental/air/cmd/snapshot_git.go @@ -62,6 +62,26 @@ func (g gitRepo) isRepository(ctx context.Context) bool { return strings.TrimSpace(out) == "true" } +// repoRelativePrefix returns the path from the repository root to g.path. Git +// emits a trailing slash for subdirectories and an empty string at the root. +func (g gitRepo) repoRelativePrefix(ctx context.Context) (string, error) { + out, err := g.run(ctx, "rev-parse", "--show-prefix") + if err != nil { + return "", fmt.Errorf("failed to resolve repository-relative path for %s: %w", g.path, err) + } + return strings.TrimRight(strings.TrimSpace(out), "/"), nil +} + +// repositoryRoot returns the top-level directory of the work tree containing +// g.path. +func (g gitRepo) repositoryRoot(ctx context.Context) (string, error) { + out, err := g.run(ctx, "rev-parse", "--show-toplevel") + if err != nil { + return "", fmt.Errorf("failed to resolve repository root for %s: %w", g.path, err) + } + return strings.TrimSpace(out), nil +} + // headSHA returns the current HEAD commit SHA. func (g gitRepo) headSHA(ctx context.Context) (string, error) { out, err := g.run(ctx, "rev-parse", "HEAD") diff --git a/experimental/air/cmd/snapshot_git_test.go b/experimental/air/cmd/snapshot_git_test.go index 9c751b7b7e0..b772c286153 100644 --- a/experimental/air/cmd/snapshot_git_test.go +++ b/experimental/air/cmd/snapshot_git_test.go @@ -65,6 +65,26 @@ func TestGitRepo_IsRepository(t *testing.T) { assert.False(t, newGitRepo(t.TempDir()).isRepository(ctx)) } +func TestGitRepo_RepositoryLayout(t *testing.T) { + ctx := t.Context() + repo := newTestRepo(t) + writeRepoFile(t, repo, "a/b/train.py", "print()") + + prefix, err := newGitRepo(repo).repoRelativePrefix(ctx) + require.NoError(t, err) + assert.Empty(t, prefix) + + subdir := filepath.Join(repo, "a", "b") + g := newGitRepo(subdir) + prefix, err = g.repoRelativePrefix(ctx) + require.NoError(t, err) + assert.Equal(t, "a/b", prefix) + + root, err := g.repositoryRoot(ctx) + require.NoError(t, err) + assert.Equal(t, repo, root) +} + func TestGitRepo_HeadSHA(t *testing.T) { ctx := t.Context() repo := newTestRepo(t) diff --git a/experimental/air/cmd/snapshot_package.go b/experimental/air/cmd/snapshot_package.go index c22f7dd61af..e39ccb38c23 100644 --- a/experimental/air/cmd/snapshot_package.go +++ b/experimental/air/cmd/snapshot_package.go @@ -17,18 +17,29 @@ import ( // to /databricks/code_source/ — so the --prefix / `-C parent dir` forms preserve it. // createGitArchiveSnapshot writes a gzipped tar of commitSHA to outputTarball via -// `git archive`, with every entry prefixed by directoryName/. When includePaths is -// set, only those paths are archived. -func createGitArchiveSnapshot(ctx context.Context, git gitRepo, commitSHA, outputTarball, directoryName string, includePaths []string) error { +// `git archive`, with every entry prefixed by directoryName/. When subtreePrefix is +// set, only that repository subtree is archived and includePaths are relative to it. +func createGitArchiveSnapshot(ctx context.Context, git gitRepo, commitSHA, outputTarball, directoryName string, includePaths []string, subtreePrefix string) error { + treeish := commitSHA + archiveGit := git + if subtreePrefix != "" { + repoRoot, err := git.repositoryRoot(ctx) + if err != nil { + return err + } + archiveGit = newGitRepo(repoRoot) + treeish = commitSHA + ":" + subtreePrefix + } + args := []string{ "archive", "--format=tar.gz", "--prefix=" + directoryName + "/", "-o", outputTarball, - commitSHA, + treeish, } args = append(args, includePaths...) - if _, err := git.run(ctx, args...); err != nil { + if _, err := archiveGit.run(ctx, args...); err != nil { return fmt.Errorf("failed to create git archive: %w", err) } return nil diff --git a/experimental/air/cmd/snapshot_package_test.go b/experimental/air/cmd/snapshot_package_test.go index 41503fd683c..9c4f07c8794 100644 --- a/experimental/air/cmd/snapshot_package_test.go +++ b/experimental/air/cmd/snapshot_package_test.go @@ -46,7 +46,7 @@ func TestCreateGitArchiveSnapshot(t *testing.T) { out := filepath.Join(t.TempDir(), "snap.tar.gz") dirName := filepath.Base(repo) - require.NoError(t, createGitArchiveSnapshot(ctx, newGitRepo(repo), sha, out, dirName, nil)) + require.NoError(t, createGitArchiveSnapshot(ctx, newGitRepo(repo), sha, out, dirName, nil, "")) entries := tarballEntries(t, out) // Every real entry is prefixed with the directory name. git archive also emits a @@ -70,13 +70,58 @@ func TestCreateGitArchiveSnapshot_IncludePaths(t *testing.T) { out := filepath.Join(t.TempDir(), "snap.tar.gz") dirName := filepath.Base(repo) - require.NoError(t, createGitArchiveSnapshot(ctx, newGitRepo(repo), sha, out, dirName, []string{"src"})) + require.NoError(t, createGitArchiveSnapshot(ctx, newGitRepo(repo), sha, out, dirName, []string{"src"}, "")) entries := tarballEntries(t, out) assert.Contains(t, entries, dirName+"/src/model.py") assert.NotContains(t, entries, dirName+"/a.txt") } +func TestCreateGitArchiveSnapshot_SubdirectoryRootPath(t *testing.T) { + ctx := t.Context() + repo := newTestRepo(t) + writeRepoFile(t, repo, "README.md", "repo root") + writeRepoFile(t, repo, "subpkg/train.py", "print()") + writeRepoFile(t, repo, "subpkg/nested/util.py", "pass") + sha := commitAll(t, repo, "init") + + rootPath := filepath.Join(repo, "subpkg") + prefix, err := newGitRepo(rootPath).repoRelativePrefix(ctx) + require.NoError(t, err) + + out := filepath.Join(t.TempDir(), "snap.tar.gz") + require.NoError(t, createGitArchiveSnapshot(ctx, newGitRepo(rootPath), sha, out, "subpkg", nil, prefix)) + + entries := tarballEntries(t, out) + assert.Contains(t, entries, "subpkg/train.py") + assert.Contains(t, entries, "subpkg/nested/util.py") + assert.NotContains(t, entries, "subpkg/README.md") + for _, entry := range entries { + assert.False(t, strings.HasPrefix(entry, "subpkg/subpkg/"), "entry %q is double nested", entry) + } +} + +func TestCreateGitArchiveSnapshot_SubdirectoryRootPathWithIncludePaths(t *testing.T) { + ctx := t.Context() + repo := newTestRepo(t) + writeRepoFile(t, repo, "subpkg/train.py", "print()") + writeRepoFile(t, repo, "subpkg/src/model.py", "pass") + writeRepoFile(t, repo, "subpkg/configs/train.yaml", "x") + sha := commitAll(t, repo, "init") + + rootPath := filepath.Join(repo, "subpkg") + prefix, err := newGitRepo(rootPath).repoRelativePrefix(ctx) + require.NoError(t, err) + + out := filepath.Join(t.TempDir(), "snap.tar.gz") + require.NoError(t, createGitArchiveSnapshot(ctx, newGitRepo(rootPath), sha, out, "subpkg", []string{"src"}, prefix)) + + entries := tarballEntries(t, out) + assert.Contains(t, entries, "subpkg/src/model.py") + assert.NotContains(t, entries, "subpkg/train.py") + assert.NotContains(t, entries, "subpkg/configs/train.yaml") +} + func TestCreatePlainTarball(t *testing.T) { ctx := t.Context() repo := t.TempDir() diff --git a/experimental/air/cmd/snapshot_resolve.go b/experimental/air/cmd/snapshot_resolve.go index 1146b641b92..dae414ca201 100644 --- a/experimental/air/cmd/snapshot_resolve.go +++ b/experimental/air/cmd/snapshot_resolve.go @@ -16,7 +16,8 @@ type snapshotMode int const ( // modeGitArchive packages a pinned commit via `git archive`. The commit is - // deterministic, so the tarball is cacheable by (commit, include_paths). + // deterministic, so the tarball is cacheable by (commit, include_paths, + // root_path subtree). modeGitArchive snapshotMode = iota // modePlainTar packages the working tree (including uncommitted changes) via // `tar`. Not cacheable — working-tree content isn't pinned to a SHA. @@ -27,11 +28,12 @@ const ( // the commit SHA to archive (git_archive only; empty for plain_tar), and whether // the working tree under the snapshot root has uncommitted changes. type snapshotPlan struct { - mode snapshotMode - commitSHA string - hasUncommit bool - isGitRepo bool - includePaths []string + mode snapshotMode + commitSHA string + hasUncommit bool + isGitRepo bool + includePaths []string + subtreePrefix string } // resolveSnapshotPlan decides how to package the snapshot (local-only): @@ -108,6 +110,12 @@ func resolveSnapshotPlan(ctx context.Context, git gitRepo, ref *gitRef, includeP return snapshotPlan{}, errors.New("git: must specify either 'branch' or 'commit'") } + subtreePrefix, err := git.repoRelativePrefix(ctx) + if err != nil { + return snapshotPlan{}, err + } + plan.subtreePrefix = subtreePrefix + // For git_archive with include_paths, verify each path exists at the resolved // commit so a typo fails fast rather than producing an empty subtree. if len(includePaths) > 0 { diff --git a/experimental/air/cmd/snapshot_resolve_test.go b/experimental/air/cmd/snapshot_resolve_test.go index c8c946f8394..1bb55c4dc17 100644 --- a/experimental/air/cmd/snapshot_resolve_test.go +++ b/experimental/air/cmd/snapshot_resolve_test.go @@ -1,6 +1,7 @@ package aircmd import ( + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -112,3 +113,16 @@ func TestResolveSnapshotPlan_IncludePaths(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "missing") } + +func TestResolveSnapshotPlan_SubdirectoryRootPath(t *testing.T) { + ctx := t.Context() + repo := newTestRepo(t) + writeRepoFile(t, repo, "subpkg/src/model.py", "1") + sha := commitAll(t, repo, "init") + + rootPath := filepath.Join(repo, "subpkg") + plan, err := resolveSnapshotPlan(ctx, newGitRepo(rootPath), &gitRef{Commit: new(sha)}, []string{"src"}) + require.NoError(t, err) + assert.Equal(t, modeGitArchive, plan.mode) + assert.Equal(t, "subpkg", plan.subtreePrefix) +} From dbfd8ecc98befbf19e2f998c91c55274e0d434d5 Mon Sep 17 00:00:00 2001 From: "ben.hansen" Date: Wed, 9 Sep 2026 17:15:07 +0000 Subject: [PATCH 2/3] Validate pinned AIR snapshot subtrees --- experimental/air/cmd/snapshot_git.go | 17 ++++++++++++++ experimental/air/cmd/snapshot_git_test.go | 22 +++++++++++++++++++ experimental/air/cmd/snapshot_resolve.go | 3 +++ experimental/air/cmd/snapshot_resolve_test.go | 14 ++++++++++++ 4 files changed, 56 insertions(+) diff --git a/experimental/air/cmd/snapshot_git.go b/experimental/air/cmd/snapshot_git.go index 4df4700111b..e0b6a98fcb7 100644 --- a/experimental/air/cmd/snapshot_git.go +++ b/experimental/air/cmd/snapshot_git.go @@ -184,6 +184,23 @@ func (g gitRepo) mergeBaseWithUpstream(ctx context.Context, remoteName string) s return "" } +// validateSubtreeExists checks that subtreePrefix is a directory at commitSHA. +func (g gitRepo) validateSubtreeExists(ctx context.Context, commitSHA, subtreePrefix string) error { + if subtreePrefix == "" { + return nil + } + + treeish := commitSHA + ":" + subtreePrefix + out, err := g.run(ctx, "cat-file", "-t", treeish) + if err != nil { + return fmt.Errorf("root_path %q does not exist at commit %s: %w", subtreePrefix, shortSHA(commitSHA), err) + } + if strings.TrimSpace(out) != "tree" { + return fmt.Errorf("root_path %q is not a directory at commit %s", subtreePrefix, shortSHA(commitSHA)) + } + return nil +} + // validateIncludePathsExist checks that every include path exists at commitSHA. // `git ls-tree` (without -d, so both blobs and trees count) reports an entry when the // path exists; empty output means missing. diff --git a/experimental/air/cmd/snapshot_git_test.go b/experimental/air/cmd/snapshot_git_test.go index b772c286153..a0591b0a071 100644 --- a/experimental/air/cmd/snapshot_git_test.go +++ b/experimental/air/cmd/snapshot_git_test.go @@ -85,6 +85,12 @@ func TestGitRepo_RepositoryLayout(t *testing.T) { assert.Equal(t, repo, root) } +func TestGitRepo_RepoRelativePrefixFailure(t *testing.T) { + _, err := newGitRepo(t.TempDir()).repoRelativePrefix(t.Context()) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to resolve repository-relative path") +} + func TestGitRepo_HeadSHA(t *testing.T) { ctx := t.Context() repo := newTestRepo(t) @@ -216,6 +222,22 @@ func TestGitRepo_ValidateIncludePathsExist(t *testing.T) { assert.Contains(t, err.Error(), sha[:8]) } +func TestGitRepo_ValidateSubtreeExists(t *testing.T) { + ctx := t.Context() + repo := newTestRepo(t) + writeRepoFile(t, repo, "subpkg/train.py", "print()") + sha := commitAll(t, repo, "init") + g := newGitRepo(filepath.Join(repo, "subpkg")) + + require.NoError(t, g.validateSubtreeExists(ctx, sha, "subpkg")) + require.NoError(t, g.validateSubtreeExists(ctx, sha, "")) + + err := g.validateSubtreeExists(ctx, sha, "missing") + require.Error(t, err) + assert.Contains(t, err.Error(), `root_path "missing" does not exist`) + assert.Contains(t, err.Error(), sha[:8]) +} + func TestBuildGitStateSidecar_PlainTarClean(t *testing.T) { ctx := t.Context() repo := newTestRepo(t) diff --git a/experimental/air/cmd/snapshot_resolve.go b/experimental/air/cmd/snapshot_resolve.go index dae414ca201..d799dd96a77 100644 --- a/experimental/air/cmd/snapshot_resolve.go +++ b/experimental/air/cmd/snapshot_resolve.go @@ -115,6 +115,9 @@ func resolveSnapshotPlan(ctx context.Context, git gitRepo, ref *gitRef, includeP return snapshotPlan{}, err } plan.subtreePrefix = subtreePrefix + if err := git.validateSubtreeExists(ctx, plan.commitSHA, subtreePrefix); err != nil { + return snapshotPlan{}, err + } // For git_archive with include_paths, verify each path exists at the resolved // commit so a typo fails fast rather than producing an empty subtree. diff --git a/experimental/air/cmd/snapshot_resolve_test.go b/experimental/air/cmd/snapshot_resolve_test.go index 1bb55c4dc17..5bce4e1bbdc 100644 --- a/experimental/air/cmd/snapshot_resolve_test.go +++ b/experimental/air/cmd/snapshot_resolve_test.go @@ -126,3 +126,17 @@ func TestResolveSnapshotPlan_SubdirectoryRootPath(t *testing.T) { assert.Equal(t, modeGitArchive, plan.mode) assert.Equal(t, "subpkg", plan.subtreePrefix) } + +func TestResolveSnapshotPlan_SubdirectoryMissingAtCommit(t *testing.T) { + ctx := t.Context() + repo := newTestRepo(t) + writeRepoFile(t, repo, "README.md", "root") + sha := commitAll(t, repo, "before subpkg") + writeRepoFile(t, repo, "subpkg/train.py", "print()") + + rootPath := filepath.Join(repo, "subpkg") + _, err := resolveSnapshotPlan(ctx, newGitRepo(rootPath), &gitRef{Commit: new(sha)}, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), `root_path "subpkg" does not exist`) + assert.Contains(t, err.Error(), sha[:8]) +} From 4df86e7ff2836ab1e26a35b8566fc2aecbf78e79 Mon Sep 17 00:00:00 2001 From: "ben.hansen" Date: Wed, 9 Sep 2026 17:46:16 +0000 Subject: [PATCH 3/3] Preserve whitespace in AIR snapshot prefixes --- experimental/air/cmd/snapshot_git.go | 4 +++- experimental/air/cmd/snapshot_git_test.go | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/experimental/air/cmd/snapshot_git.go b/experimental/air/cmd/snapshot_git.go index e0b6a98fcb7..6172ccec9c6 100644 --- a/experimental/air/cmd/snapshot_git.go +++ b/experimental/air/cmd/snapshot_git.go @@ -69,7 +69,9 @@ func (g gitRepo) repoRelativePrefix(ctx context.Context) (string, error) { if err != nil { return "", fmt.Errorf("failed to resolve repository-relative path for %s: %w", g.path, err) } - return strings.TrimRight(strings.TrimSpace(out), "/"), nil + out = strings.TrimSuffix(out, "\n") + out = strings.TrimSuffix(out, "\r") + return strings.TrimSuffix(out, "/"), nil } // repositoryRoot returns the top-level directory of the work tree containing diff --git a/experimental/air/cmd/snapshot_git_test.go b/experimental/air/cmd/snapshot_git_test.go index a0591b0a071..7096e5ad0b0 100644 --- a/experimental/air/cmd/snapshot_git_test.go +++ b/experimental/air/cmd/snapshot_git_test.go @@ -83,6 +83,11 @@ func TestGitRepo_RepositoryLayout(t *testing.T) { root, err := g.repositoryRoot(ctx) require.NoError(t, err) assert.Equal(t, repo, root) + + writeRepoFile(t, repo, " leading-space/train.py", "print()") + prefix, err = newGitRepo(filepath.Join(repo, " leading-space")).repoRelativePrefix(ctx) + require.NoError(t, err) + assert.Equal(t, " leading-space", prefix) } func TestGitRepo_RepoRelativePrefixFailure(t *testing.T) {