fix(mirror): isolate downstream tag refs

AGENTS.md

@@ -7,6 +7,8 @@
 
 ## Git And Tests
 
+- All changes must land through pull requests; do not push changes directly to
+  the default branch.
 - Product code that invokes Git should stay behind `internal/gitexec`.
 - Tests must not depend on the user's global Git identity, real Braid cache, or
   network remotes.

internal/command/add_test.go

@@ -432,7 +432,7 @@ func TestAddCommandBlocksUnresolvedGitOperationBeforeScopedStatus(t *testing.T)
 	assertContains(t, stderr, "unresolved git operation state is present: MERGE_HEAD")
 }
 
-func TestAddCommandNoCacheTags(t *testing.T) {
+func TestAddCommandTags(t *testing.T) {
 	tests := []struct {
 		name string
 		tag  string
@@ -463,23 +463,47 @@ func TestAddCommandNoCacheTags(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			upstream := testutil.InitRepo(t)
-			revision := test.make(t, upstream, test.tag)
-			repo := initDownstream(t)
-			localPath := "vendor/" + test.name
-			runCommandOK(t, repo, []string{"--no-cache", "add", upstream, localPath, "--tag", test.tag})
+		for _, mode := range []string{"no-cache", "global-cache"} {
+			t.Run(test.name+"/"+mode, func(t *testing.T) {
+				upstream := testutil.InitRepo(t)
+				revision := test.make(t, upstream, test.tag)
+				repo := initDownstream(t)
+				localPath := "vendor/" + test.name
+				args := []string{"--no-cache", "add", upstream, localPath, "--tag", test.tag}
+				if mode == "global-cache" {
+					args = []string{"--global-cache-dir", t.TempDir(), "add", upstream, localPath, "--tag", test.tag}
+				}
+				runCommandOK(t, repo, args)
 
-			assertFile(t, repo, localPath+"/README.md", test.name+"\n")
-			m := loadMirror(t, repo, localPath)
-			if m.Tag != test.tag || m.Branch != "" || m.Revision != revision {
-				t.Fatalf("mirror = %#v, want tag %q at %s", m, test.tag, revision)
-			}
-			assertClean(t, repo)
-		})
+				assertFile(t, repo, localPath+"/README.md", test.name+"\n")
+				m := loadMirror(t, repo, localPath)
+				if m.Tag != test.tag || m.Branch != "" || m.Revision != revision {
+					t.Fatalf("mirror = %#v, want tag %q at %s", m, test.tag, revision)
+				}
+				git := gitexec.New(repo, false, nil)
+				assertRefMissing(t, context.Background(), git, "refs/tags/"+test.tag)
+				assertRefMissing(t, context.Background(), git, m.LocalRef())
+				assertClean(t, repo)
+			})
+		}
 	}
 }
 
+func TestAddTagMirrorPreservesSameNamedDownstreamTag(t *testing.T) {
+	upstream := testutil.InitRepo(t)
+	testutil.WriteFile(t, upstream, "README.md", "upstream\n")
+	testutil.CommitAll(t, upstream, "upstream")
+	testutil.Git(t, upstream, "tag", "v1")
+
+	repo := initDownstream(t)
+	testutil.Git(t, repo, "tag", "-a", "v1", "-m", "downstream tag")
+	downstreamTag := strings.TrimSpace(testutil.Git(t, repo, "rev-parse", "refs/tags/v1").Stdout)
+
+	runCommandOK(t, repo, []string{"--no-cache", "add", upstream, "vendor/tagged", "--tag", "v1"})
+
+	assertRefObjectID(t, context.Background(), gitexec.New(repo, false, nil), "refs/tags/v1", downstreamTag)
+}
+
 func TestAddCommandMissingUpstreamPathCleansUpTemporaryRemote(t *testing.T) {
 	upstream := testutil.InitRepo(t)
 	testutil.WriteFile(t, upstream, "README.md", "hello\n")

internal/command/cache.go

@@ -242,7 +242,7 @@ func fetchMirror(ctx context.Context, git fetchGit, cache CacheConfig, m mirror.
 					args = append(args, "+refs/heads/"+m.Branch+":refs/remotes/"+m.Remote()+"/"+m.Branch)
 				}
 				if m.Tag != "" {
-					args = append(args, "+refs/tags/"+m.Tag+":refs/tags/"+m.Tag)
+					args = append(args, "+refs/tags/"+m.Tag+":"+m.LocalRef())
 				}
 				args = append(args, "+refs/braid/*:refs/braid/*")
 				return git.Fetch(ctx, args...)
@@ -251,7 +251,7 @@ func fetchMirror(ctx context.Context, git fetchGit, cache CacheConfig, m mirror.
 				return err
 			}
 			if m.Tag != "" {
-				return git.Fetch(ctx, "-n", m.Remote(), "+refs/tags/"+m.Tag+":refs/tags/"+m.Tag)
+				return git.Fetch(ctx, "-n", m.Remote(), "+refs/tags/"+m.Tag+":"+m.LocalRef())
 			}
 			return nil
 		},

internal/command/cache_test.go

@@ -296,6 +296,31 @@ func TestFetchMirrorFromRepositoryLocalCacheFetchesBraidRefs(t *testing.T) {
 	}
 }
 
+func TestFetchTagMirrorFromRepositoryLocalCacheUsesRemoteTrackingRef(t *testing.T) {
+	ctx := context.Background()
+	upstream := testutil.InitRepo(t)
+	testutil.WriteFile(t, upstream, "README.md", "base\n")
+	revision := testutil.CommitAll(t, upstream, "base")
+	testutil.Git(t, upstream, "tag", "v1")
+	repo := testutil.InitRepo(t)
+	cache := repositoryLocalCacheForTest(t, ctx, repo)
+	m := mirror.Mirror{Path: "vendor/basic", URL: upstream, Tag: "v1", Revision: revision}
+
+	if err := (MirrorObjectCache{Config: cache}).Hydrate(ctx, m); err != nil {
+		t.Fatalf("Hydrate returned error: %v", err)
+	}
+	git := gitexec.New(repo, false, nil)
+	if err := setupOne(ctx, git, m, true, cache); err != nil {
+		t.Fatalf("setupOne returned error: %v", err)
+	}
+	if err := fetchMirror(ctx, git, cache, m, progressReporter{}); err != nil {
+		t.Fatalf("fetchMirror returned error: %v", err)
+	}
+
+	assertRefCommit(t, ctx, git, m.LocalRef(), revision)
+	assertRefMissing(t, ctx, git, "refs/tags/v1")
+}
+
 func TestAcquireCacheLockTimesOut(t *testing.T) {
 	lockPath := filepath.Join(t.TempDir(), "mirror.git.lock")
 	if err := os.Mkdir(lockPath, 0o700); err != nil {
@@ -325,17 +350,6 @@ func repositoryLocalCacheForTest(t *testing.T, ctx context.Context, repo string)
 	return cache
 }
 
-func assertRefCommit(t *testing.T, ctx context.Context, git gitexec.Git, ref, want string) {
-	t.Helper()
-	got, err := git.RevParse(ctx, ref+"^{commit}")
-	if err != nil {
-		t.Fatalf("rev-parse %s: %v", ref, err)
-	}
-	if got != want {
-		t.Fatalf("%s = %s, want %s", ref, got, want)
-	}
-}
-
 func assertPathIsDir(t *testing.T, path string) {
 	t.Helper()
 	info, err := os.Stat(path)

internal/command/sync_test.go

@@ -1106,6 +1106,28 @@ func TestSyncCommandKeepRetainsTemporaryRemote(t *testing.T) {
 	assertGitRemote(t, repo, "main_braid_vendor_basic")
 }
 
+func TestSyncCommandKeepRetainsDistinctTagTrackingRefs(t *testing.T) {
+	ctx := context.Background()
+	upstreamA := testutil.InitRepo(t)
+	testutil.WriteFile(t, upstreamA, "README.md", "a\n")
+	revisionA := testutil.CommitAll(t, upstreamA, "a")
+	testutil.Git(t, upstreamA, "tag", "v1")
+	upstreamB := testutil.InitRepo(t)
+	testutil.WriteFile(t, upstreamB, "README.md", "b\n")
+	revisionB := testutil.CommitAll(t, upstreamB, "b")
+	testutil.Git(t, upstreamB, "tag", "v1")
+	repo := initDownstream(t)
+	runCommandOK(t, repo, []string{"add", upstreamA, "vendor/a", "--tag", "v1"})
+	runCommandOK(t, repo, []string{"add", upstreamB, "vendor/b", "--tag", "v1"})
+
+	runCommandOK(t, repo, []string{"sync", "--pull-only", "--keep", "vendor/a", "vendor/b"})
+
+	git := gitexec.New(repo, false, nil)
+	assertRefCommit(t, ctx, git, "refs/remotes/v1_braid_vendor_a/tags/v1", revisionA)
+	assertRefCommit(t, ctx, git, "refs/remotes/v1_braid_vendor_b/tags/v1", revisionB)
+	assertRefMissing(t, ctx, git, "refs/tags/v1")
+}
+
 func assertNoGitRemote(t *testing.T, repo, remote string) {
 	t.Helper()
 	remotes := strings.Fields(testutil.Git(t, repo, "remote").Stdout)

internal/command/test_support_test.go

@@ -24,6 +24,35 @@ func initDownstream(t *testing.T) string {
 	return repo
 }
 
+func assertRefCommit(t *testing.T, ctx context.Context, git gitexec.Git, ref, want string) {
+	t.Helper()
+	got, err := git.RevParse(ctx, ref+"^{commit}")
+	if err != nil {
+		t.Fatalf("rev-parse %s: %v", ref, err)
+	}
+	if got != want {
+		t.Fatalf("%s = %s, want %s", ref, got, want)
+	}
+}
+
+func assertRefObjectID(t *testing.T, ctx context.Context, git gitexec.Git, ref, want string) {
+	t.Helper()
+	got, err := git.RevParse(ctx, ref)
+	if err != nil {
+		t.Fatalf("rev-parse %s: %v", ref, err)
+	}
+	if got != want {
+		t.Fatalf("%s = %s, want %s", ref, got, want)
+	}
+}
+
+func assertRefMissing(t *testing.T, ctx context.Context, git gitexec.Git, ref string) {
+	t.Helper()
+	if result, err := git.RunOK(ctx, "rev-parse", "--verify", "--quiet", ref); err == nil {
+		t.Fatalf("ref %s exists unexpectedly at %s", ref, strings.TrimSpace(result.Stdout))
+	}
+}
+
 func runCommandOK(t *testing.T, repo string, args []string) string {
 	t.Helper()
 	return runCommandOKInDir(t, repo, repo, args)

internal/command/update_test.go

@@ -1042,6 +1042,8 @@ func TestUpdateCommandNoCacheTags(t *testing.T) {
 				testutil.Git(t, upstream, "tag", test.tag)
 			}
 			repo := initDownstream(t)
+			testutil.Git(t, repo, "tag", "-a", test.tag, "-m", "downstream tag")
+			downstreamTag := strings.TrimSpace(testutil.Git(t, repo, "rev-parse", "refs/tags/"+test.tag).Stdout)
 			localPath := "vendor/" + test.name
 			runCommandOK(t, repo, []string{"--no-cache", "add", upstream, localPath, "--tag", test.tag})
 
@@ -1051,6 +1053,7 @@ func TestUpdateCommandNoCacheTags(t *testing.T) {
 			if got := loadMirror(t, repo, localPath).Revision; got != revision {
 				t.Fatalf("revision = %q, want %q", got, revision)
 			}
+			assertRefObjectID(t, context.Background(), gitexec.New(repo, false, nil), "refs/tags/"+test.tag, downstreamTag)
 		})
 	}
 }

internal/mirror/mirror.go

@@ -74,7 +74,7 @@ func (m Mirror) LocalRef() string {
 	case m.Branch != "":
 		return m.Remote() + "/" + m.Branch
 	case m.Tag != "":
-		return "tags/" + m.Tag
+		return "refs/remotes/" + m.Remote() + "/tags/" + m.Tag
 	default:
 		return m.Revision
 	}

internal/mirror/mirror_test.go

@@ -64,7 +64,7 @@ func TestMirrorRefsAndRemote(t *testing.T) {
 	if got, want := tag.Remote(), "v1_braid_mytool"; got != want {
 		t.Fatalf("Remote = %q, want %q", got, want)
 	}
-	if got, want := tag.LocalRef(), "tags/v1"; got != want {
+	if got, want := tag.LocalRef(), "refs/remotes/v1_braid_mytool/tags/v1"; got != want {
 		t.Fatalf("LocalRef = %q, want %q", got, want)
 	}
 	if got, err := tag.RemoteRef(); err != nil || got != "+refs/tags/v1" {