From 6ce7cf3ba4110096d82614b0da617a1c2826c7b1 Mon Sep 17 00:00:00 2001 From: Peter Donald Date: Sat, 11 Jul 2026 15:00:51 +1000 Subject: [PATCH 1/3] fix(mirror): isolate downstream tag refs Fetch tag mirrors into mirror-specific remote-tracking refs so importing upstream content cannot create or overwrite downstream tags. Cover cache modes, cleanup, retained remotes, and exact tag-object preservation. --- AGENTS.md | 2 + internal/command/add_test.go | 52 +++++++--- internal/command/cache.go | 4 +- internal/command/cache_test.go | 36 ++++--- internal/command/sync_test.go | 22 +++++ internal/command/test_support_test.go | 29 ++++++ internal/command/update_test.go | 3 + internal/mirror/mirror.go | 2 +- internal/mirror/mirror_test.go | 2 +- plans/tag-mirror-refs/00-requirements.md | 59 ++++++++++++ .../tag-mirror-refs/10-implementation-plan.md | 59 ++++++++++++ plans/tag-mirror-refs/20-task-board.yaml | 96 +++++++++++++++++++ 12 files changed, 337 insertions(+), 29 deletions(-) create mode 100644 plans/tag-mirror-refs/00-requirements.md create mode 100644 plans/tag-mirror-refs/10-implementation-plan.md create mode 100644 plans/tag-mirror-refs/20-task-board.yaml diff --git a/AGENTS.md b/AGENTS.md index 8e2ba42..1b58a77 100644 --- a/AGENTS.md +++ b/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. diff --git a/internal/command/add_test.go b/internal/command/add_test.go index ec07196..59c30b9 100644 --- a/internal/command/add_test.go +++ b/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") diff --git a/internal/command/cache.go b/internal/command/cache.go index 6916677..025ca55 100644 --- a/internal/command/cache.go +++ b/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 }, diff --git a/internal/command/cache_test.go b/internal/command/cache_test.go index f21d32c..d881e1a 100644 --- a/internal/command/cache_test.go +++ b/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) diff --git a/internal/command/sync_test.go b/internal/command/sync_test.go index 28e872d..6a61773 100644 --- a/internal/command/sync_test.go +++ b/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) diff --git a/internal/command/test_support_test.go b/internal/command/test_support_test.go index 038e63a..ba2d948 100644 --- a/internal/command/test_support_test.go +++ b/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) diff --git a/internal/command/update_test.go b/internal/command/update_test.go index e206ef5..4068b6e 100644 --- a/internal/command/update_test.go +++ b/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) }) } } diff --git a/internal/mirror/mirror.go b/internal/mirror/mirror.go index 10c7027..771a9da 100644 --- a/internal/mirror/mirror.go +++ b/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 } diff --git a/internal/mirror/mirror_test.go b/internal/mirror/mirror_test.go index 6fc0c7d..06e3bc3 100644 --- a/internal/mirror/mirror_test.go +++ b/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" { diff --git a/plans/tag-mirror-refs/00-requirements.md b/plans/tag-mirror-refs/00-requirements.md new file mode 100644 index 0000000..d46be88 --- /dev/null +++ b/plans/tag-mirror-refs/00-requirements.md @@ -0,0 +1,59 @@ +# Tag Mirror Ref Isolation Requirements + +## Problem + +Tag mirrors currently fetch `refs/tags/` into the downstream repository's +global `refs/tags/` namespace. Importing mirror content must not create or +overwrite a downstream tag. + +## Scope + +In scope: + +- Fetch tag mirrors into a mirror-specific remote-tracking ref. +- Preserve lightweight and annotated tag resolution to the peeled commit. +- Cover normal, global-cache, and repository-local-cache command paths. +- Preserve `--keep`: a retained Braid remote keeps its tracking ref. +- Add a repository instruction requiring changes to land through pull requests. +- Deliver the implementation on a new branch through a pull request. + +Out of scope: + +- Deleting downstream tags created by earlier Braid versions. +- Changing branch or revision mirror behavior. +- Changing the global or repository-local cache's internal tag namespace; the + defect concerns refs in the downstream repository. + +## Behavior Requirements + +1. A tag mirror's downstream fetch refspec is + `+refs/tags/:refs/remotes//tags/`. +2. `Mirror.LocalRef()` returns the full + `refs/remotes//tags/` ref for tag mirrors; resolution uses + that ref and continues to peel annotated tags to commits. +3. Normal remote cleanup removes the mirror-specific tracking ref. +4. `--keep` retains the remote and its mirror-specific tracking ref. +5. Braid never creates, updates, or deletes `refs/tags/` in the downstream + repository as part of this change. +6. Existing downstream tags remain untouched, including tags that may have been + created by an earlier Braid version. + +## Acceptance Criteria + +- [ ] Adding and updating lightweight and annotated tag mirrors works in every + cache mode without creating a downstream tag. +- [ ] Two tag mirrors with the same tag name do not share a downstream ref. +- [ ] A pre-existing, same-named downstream tag retains its original object ID. +- [ ] Normal cleanup leaves neither the temporary remote nor its tracking ref. +- [ ] `--keep` retains the mirror-specific tracking ref and no global tag. +- [ ] `AGENTS.md` states that changes must land through pull requests. +- [ ] All CI-parity checks pass and the branch is published as a PR. + +## Decisions + +- Q-001 resolved: use a mirror-specific remote-tracking ref and remove it with + normal temporary-remote cleanup. +- Q-002 resolved: do not clean up previously created downstream tags because + ownership cannot be determined safely. +- Q-003 resolved: retain the mirror-specific tag ref when `--keep` retains the + remote. diff --git a/plans/tag-mirror-refs/10-implementation-plan.md b/plans/tag-mirror-refs/10-implementation-plan.md new file mode 100644 index 0000000..5d90ed4 --- /dev/null +++ b/plans/tag-mirror-refs/10-implementation-plan.md @@ -0,0 +1,59 @@ +# Tag Mirror Ref Isolation Implementation Plan + +## Phase Sequence + +1. Add regression assertions that tag add/update paths do not populate the + downstream global tag namespace, do not change a pre-existing same-named + downstream tag, and retain isolated tracking refs only when requested. +2. Change tag `LocalRef` and downstream tag fetch refspecs to use + `refs/remotes//tags/` consistently, including the + repository-local-cache fetch path. +3. Add the PR-only delivery non-negotiable to `AGENTS.md` and update user-facing + documentation only if the implementation exposes a contract worth stating. +4. Run targeted tests followed by every CI-parity check, inspect the diff, + commit, push, and open a ready pull request. + +## Expected Files + +- `internal/mirror/mirror.go` +- `internal/mirror/mirror_test.go` +- `internal/command/cache.go` +- focused command or integration tests for tag ref behavior +- `AGENTS.md` + +## High-Risk Areas + +- Annotated tag peeling: + - Impact: resolving the tracking ref incorrectly could import a tag object + rather than its commit or fail tree extraction. + - Mitigation: retain `^{commit}` resolution and test annotated tags. +- Cache-mode divergence: + - Impact: repository-local cache has an explicit downstream refspec distinct + from the normal/global-cache path. + - Mitigation: assert behavior across no-cache, global-cache, and + repository-local-cache modes. +- Accidental deletion of user refs: + - Impact: fetching or cleanup could overwrite or delete a user-owned global + tag. + - Mitigation: test a pre-existing same-named tag by object ID; never target + `refs/tags/` in downstream fetch or cleanup operations. + +## Required Full Gates + +1. `bazel run @rules_go//go -- fmt ./...` +2. `git diff --exit-code` immediately after formatting on the committed + implementation, proving formatting introduced no changes +3. `bazel test --test_env=PATH //...` +4. `bazel run @rules_go//go -- vet ./...` +5. `bazel run @rules_go//go -- run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.0 run` +6. `bazel test --test_env=PATH //integration/...` + +## Completion Criteria + +- All acceptance criteria and tasks are complete with recorded evidence. +- All CI-parity gates pass. +- Final `git diff` and `git status` are reviewed after any plan-state-only + follow-up commit. +- The diff contains only the approved change, tests, plan state, and requested + repository instruction. +- Changes are committed, pushed, and available in a ready pull request. diff --git a/plans/tag-mirror-refs/20-task-board.yaml b/plans/tag-mirror-refs/20-task-board.yaml new file mode 100644 index 0000000..44785ac --- /dev/null +++ b/plans/tag-mirror-refs/20-task-board.yaml @@ -0,0 +1,96 @@ +meta: + project: tag-mirror-ref-isolation + last_updated: 2026-07-11 + plan_status: accepted + approved_on: 2026-07-11 + required_full_gates: + - bazel run @rules_go//go -- fmt ./... + - git diff --exit-code + - bazel test --test_env=PATH //... + - bazel run @rules_go//go -- vet ./... + - bazel run @rules_go//go -- run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.0 run + - bazel test --test_env=PATH //integration/... + +current_focus: T04 + +tasks: + - id: T01 + title: Add downstream tag-ref regression coverage + status: completed + priority: high + depends_on: [] + acceptance_criteria: + - Tag add and update coverage rejects downstream global tag creation. + - A pre-existing same-named downstream tag retains its object ID. + - Normal cleanup removes the isolated ref and --keep retains it. + - Two retained remotes tracking the same tag name have distinct refs that resolve independently. + - Lightweight and annotated tags and all cache modes are represented. + files_touched: + - internal/command/*_test.go + - integration/*_test.go + evidence: + - command: bazel test --test_env=PATH //internal/mirror:mirror_test //internal/command:add_test //internal/command:cache_test //internal/command:sync_test //internal/command:refresh_command_test + result: pass + commit: + hash: pending + message: pending + + - id: T02 + title: Isolate downstream tag mirror refs + status: completed + priority: high + depends_on: [T01] + acceptance_criteria: + - Mirror.LocalRef returns refs/remotes//tags/ for tag mirrors. + - Downstream tag fetches use +refs/tags/:refs/remotes//tags/. + - No command path writes the mirrored tag to downstream refs/tags. + - Existing downstream tags are not deleted or modified. + files_touched: + - internal/mirror/mirror.go + - internal/mirror/mirror_test.go + - internal/command/cache.go + evidence: + - command: bazel test --test_env=PATH //internal/mirror:mirror_test //internal/command:add_test //internal/command:cache_test //internal/command:sync_test //internal/command:refresh_command_test + result: pass + commit: + hash: pending + message: pending + + - id: T03 + title: Add PR-only repository instruction + status: completed + priority: high + depends_on: [] + acceptance_criteria: + - AGENTS.md contains a non-negotiable requiring changes to land through pull requests. + files_touched: + - AGENTS.md + evidence: + - command: git diff --check + result: pass + commit: + hash: pending + message: pending + + - id: T04 + title: Validate and publish + status: in_progress + priority: high + depends_on: [T02, T03] + acceptance_criteria: + - Every CI-parity command passes. + - Formatting is run on committed implementation state and git diff --exit-code then passes. + - Diff is reviewed and scoped. + - Branch is committed, pushed, and opened as a ready pull request. + files_touched: + - plans/tag-mirror-refs/20-task-board.yaml + evidence: [] + commit: + hash: pending + message: pending + +history: + - timestamp: 2026-07-11 + task: planning + commit: not_required + note: Drafted after resolving tag namespace, legacy cleanup, and --keep decisions. From 3e483d227301d6ed420bc2a993f2cca838e40be7 Mon Sep 17 00:00:00 2001 From: Peter Donald Date: Sat, 11 Jul 2026 15:02:55 +1000 Subject: [PATCH 2/3] chore(plan): record tag mirror validation --- plans/tag-mirror-refs/20-task-board.yaml | 39 ++++++++++++++++-------- 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/plans/tag-mirror-refs/20-task-board.yaml b/plans/tag-mirror-refs/20-task-board.yaml index 44785ac..26f89ab 100644 --- a/plans/tag-mirror-refs/20-task-board.yaml +++ b/plans/tag-mirror-refs/20-task-board.yaml @@ -11,7 +11,7 @@ meta: - bazel run @rules_go//go -- run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.0 run - bazel test --test_env=PATH //integration/... -current_focus: T04 +current_focus: complete tasks: - id: T01 @@ -32,8 +32,8 @@ tasks: - command: bazel test --test_env=PATH //internal/mirror:mirror_test //internal/command:add_test //internal/command:cache_test //internal/command:sync_test //internal/command:refresh_command_test result: pass commit: - hash: pending - message: pending + hash: 6ce7cf3b + message: "fix(mirror): isolate downstream tag refs" - id: T02 title: Isolate downstream tag mirror refs @@ -53,8 +53,8 @@ tasks: - command: bazel test --test_env=PATH //internal/mirror:mirror_test //internal/command:add_test //internal/command:cache_test //internal/command:sync_test //internal/command:refresh_command_test result: pass commit: - hash: pending - message: pending + hash: 6ce7cf3b + message: "fix(mirror): isolate downstream tag refs" - id: T03 title: Add PR-only repository instruction @@ -69,12 +69,12 @@ tasks: - command: git diff --check result: pass commit: - hash: pending - message: pending + hash: 6ce7cf3b + message: "fix(mirror): isolate downstream tag refs" - id: T04 title: Validate and publish - status: in_progress + status: completed priority: high depends_on: [T02, T03] acceptance_criteria: @@ -84,13 +84,28 @@ tasks: - Branch is committed, pushed, and opened as a ready pull request. files_touched: - plans/tag-mirror-refs/20-task-board.yaml - evidence: [] - commit: - hash: pending - message: pending + evidence: + - command: bazel run @rules_go//go -- fmt ./... + result: pass + - command: git diff --exit-code + result: pass + - command: bazel test --test_env=PATH //... + result: pass + - command: bazel run @rules_go//go -- vet ./... + result: pass + - command: bazel run @rules_go//go -- run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.0 run + result: pass (0 issues) + - command: bazel test --test_env=PATH //integration/... + result: pass + - command: implementation alignment review rounds 1-2 + result: pass (no remaining findings) history: - timestamp: 2026-07-11 task: planning commit: not_required note: Drafted after resolving tag namespace, legacy cleanup, and --keep decisions. + - timestamp: 2026-07-11 + task: T01-T03 + commit: 6ce7cf3b + note: Implemented isolated tag refs, regression coverage, and PR-only repository guidance. From cdded1e02ab4eb23cc3c780d5705004b9c11de02 Mon Sep 17 00:00:00 2001 From: Peter Donald Date: Sat, 11 Jul 2026 15:03:04 +1000 Subject: [PATCH 3/3] chore(plan): remove completed tag mirror plan --- plans/tag-mirror-refs/00-requirements.md | 59 ---------- .../tag-mirror-refs/10-implementation-plan.md | 59 ---------- plans/tag-mirror-refs/20-task-board.yaml | 111 ------------------ 3 files changed, 229 deletions(-) delete mode 100644 plans/tag-mirror-refs/00-requirements.md delete mode 100644 plans/tag-mirror-refs/10-implementation-plan.md delete mode 100644 plans/tag-mirror-refs/20-task-board.yaml diff --git a/plans/tag-mirror-refs/00-requirements.md b/plans/tag-mirror-refs/00-requirements.md deleted file mode 100644 index d46be88..0000000 --- a/plans/tag-mirror-refs/00-requirements.md +++ /dev/null @@ -1,59 +0,0 @@ -# Tag Mirror Ref Isolation Requirements - -## Problem - -Tag mirrors currently fetch `refs/tags/` into the downstream repository's -global `refs/tags/` namespace. Importing mirror content must not create or -overwrite a downstream tag. - -## Scope - -In scope: - -- Fetch tag mirrors into a mirror-specific remote-tracking ref. -- Preserve lightweight and annotated tag resolution to the peeled commit. -- Cover normal, global-cache, and repository-local-cache command paths. -- Preserve `--keep`: a retained Braid remote keeps its tracking ref. -- Add a repository instruction requiring changes to land through pull requests. -- Deliver the implementation on a new branch through a pull request. - -Out of scope: - -- Deleting downstream tags created by earlier Braid versions. -- Changing branch or revision mirror behavior. -- Changing the global or repository-local cache's internal tag namespace; the - defect concerns refs in the downstream repository. - -## Behavior Requirements - -1. A tag mirror's downstream fetch refspec is - `+refs/tags/:refs/remotes//tags/`. -2. `Mirror.LocalRef()` returns the full - `refs/remotes//tags/` ref for tag mirrors; resolution uses - that ref and continues to peel annotated tags to commits. -3. Normal remote cleanup removes the mirror-specific tracking ref. -4. `--keep` retains the remote and its mirror-specific tracking ref. -5. Braid never creates, updates, or deletes `refs/tags/` in the downstream - repository as part of this change. -6. Existing downstream tags remain untouched, including tags that may have been - created by an earlier Braid version. - -## Acceptance Criteria - -- [ ] Adding and updating lightweight and annotated tag mirrors works in every - cache mode without creating a downstream tag. -- [ ] Two tag mirrors with the same tag name do not share a downstream ref. -- [ ] A pre-existing, same-named downstream tag retains its original object ID. -- [ ] Normal cleanup leaves neither the temporary remote nor its tracking ref. -- [ ] `--keep` retains the mirror-specific tracking ref and no global tag. -- [ ] `AGENTS.md` states that changes must land through pull requests. -- [ ] All CI-parity checks pass and the branch is published as a PR. - -## Decisions - -- Q-001 resolved: use a mirror-specific remote-tracking ref and remove it with - normal temporary-remote cleanup. -- Q-002 resolved: do not clean up previously created downstream tags because - ownership cannot be determined safely. -- Q-003 resolved: retain the mirror-specific tag ref when `--keep` retains the - remote. diff --git a/plans/tag-mirror-refs/10-implementation-plan.md b/plans/tag-mirror-refs/10-implementation-plan.md deleted file mode 100644 index 5d90ed4..0000000 --- a/plans/tag-mirror-refs/10-implementation-plan.md +++ /dev/null @@ -1,59 +0,0 @@ -# Tag Mirror Ref Isolation Implementation Plan - -## Phase Sequence - -1. Add regression assertions that tag add/update paths do not populate the - downstream global tag namespace, do not change a pre-existing same-named - downstream tag, and retain isolated tracking refs only when requested. -2. Change tag `LocalRef` and downstream tag fetch refspecs to use - `refs/remotes//tags/` consistently, including the - repository-local-cache fetch path. -3. Add the PR-only delivery non-negotiable to `AGENTS.md` and update user-facing - documentation only if the implementation exposes a contract worth stating. -4. Run targeted tests followed by every CI-parity check, inspect the diff, - commit, push, and open a ready pull request. - -## Expected Files - -- `internal/mirror/mirror.go` -- `internal/mirror/mirror_test.go` -- `internal/command/cache.go` -- focused command or integration tests for tag ref behavior -- `AGENTS.md` - -## High-Risk Areas - -- Annotated tag peeling: - - Impact: resolving the tracking ref incorrectly could import a tag object - rather than its commit or fail tree extraction. - - Mitigation: retain `^{commit}` resolution and test annotated tags. -- Cache-mode divergence: - - Impact: repository-local cache has an explicit downstream refspec distinct - from the normal/global-cache path. - - Mitigation: assert behavior across no-cache, global-cache, and - repository-local-cache modes. -- Accidental deletion of user refs: - - Impact: fetching or cleanup could overwrite or delete a user-owned global - tag. - - Mitigation: test a pre-existing same-named tag by object ID; never target - `refs/tags/` in downstream fetch or cleanup operations. - -## Required Full Gates - -1. `bazel run @rules_go//go -- fmt ./...` -2. `git diff --exit-code` immediately after formatting on the committed - implementation, proving formatting introduced no changes -3. `bazel test --test_env=PATH //...` -4. `bazel run @rules_go//go -- vet ./...` -5. `bazel run @rules_go//go -- run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.0 run` -6. `bazel test --test_env=PATH //integration/...` - -## Completion Criteria - -- All acceptance criteria and tasks are complete with recorded evidence. -- All CI-parity gates pass. -- Final `git diff` and `git status` are reviewed after any plan-state-only - follow-up commit. -- The diff contains only the approved change, tests, plan state, and requested - repository instruction. -- Changes are committed, pushed, and available in a ready pull request. diff --git a/plans/tag-mirror-refs/20-task-board.yaml b/plans/tag-mirror-refs/20-task-board.yaml deleted file mode 100644 index 26f89ab..0000000 --- a/plans/tag-mirror-refs/20-task-board.yaml +++ /dev/null @@ -1,111 +0,0 @@ -meta: - project: tag-mirror-ref-isolation - last_updated: 2026-07-11 - plan_status: accepted - approved_on: 2026-07-11 - required_full_gates: - - bazel run @rules_go//go -- fmt ./... - - git diff --exit-code - - bazel test --test_env=PATH //... - - bazel run @rules_go//go -- vet ./... - - bazel run @rules_go//go -- run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.0 run - - bazel test --test_env=PATH //integration/... - -current_focus: complete - -tasks: - - id: T01 - title: Add downstream tag-ref regression coverage - status: completed - priority: high - depends_on: [] - acceptance_criteria: - - Tag add and update coverage rejects downstream global tag creation. - - A pre-existing same-named downstream tag retains its object ID. - - Normal cleanup removes the isolated ref and --keep retains it. - - Two retained remotes tracking the same tag name have distinct refs that resolve independently. - - Lightweight and annotated tags and all cache modes are represented. - files_touched: - - internal/command/*_test.go - - integration/*_test.go - evidence: - - command: bazel test --test_env=PATH //internal/mirror:mirror_test //internal/command:add_test //internal/command:cache_test //internal/command:sync_test //internal/command:refresh_command_test - result: pass - commit: - hash: 6ce7cf3b - message: "fix(mirror): isolate downstream tag refs" - - - id: T02 - title: Isolate downstream tag mirror refs - status: completed - priority: high - depends_on: [T01] - acceptance_criteria: - - Mirror.LocalRef returns refs/remotes//tags/ for tag mirrors. - - Downstream tag fetches use +refs/tags/:refs/remotes//tags/. - - No command path writes the mirrored tag to downstream refs/tags. - - Existing downstream tags are not deleted or modified. - files_touched: - - internal/mirror/mirror.go - - internal/mirror/mirror_test.go - - internal/command/cache.go - evidence: - - command: bazel test --test_env=PATH //internal/mirror:mirror_test //internal/command:add_test //internal/command:cache_test //internal/command:sync_test //internal/command:refresh_command_test - result: pass - commit: - hash: 6ce7cf3b - message: "fix(mirror): isolate downstream tag refs" - - - id: T03 - title: Add PR-only repository instruction - status: completed - priority: high - depends_on: [] - acceptance_criteria: - - AGENTS.md contains a non-negotiable requiring changes to land through pull requests. - files_touched: - - AGENTS.md - evidence: - - command: git diff --check - result: pass - commit: - hash: 6ce7cf3b - message: "fix(mirror): isolate downstream tag refs" - - - id: T04 - title: Validate and publish - status: completed - priority: high - depends_on: [T02, T03] - acceptance_criteria: - - Every CI-parity command passes. - - Formatting is run on committed implementation state and git diff --exit-code then passes. - - Diff is reviewed and scoped. - - Branch is committed, pushed, and opened as a ready pull request. - files_touched: - - plans/tag-mirror-refs/20-task-board.yaml - evidence: - - command: bazel run @rules_go//go -- fmt ./... - result: pass - - command: git diff --exit-code - result: pass - - command: bazel test --test_env=PATH //... - result: pass - - command: bazel run @rules_go//go -- vet ./... - result: pass - - command: bazel run @rules_go//go -- run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.0 run - result: pass (0 issues) - - command: bazel test --test_env=PATH //integration/... - result: pass - - command: implementation alignment review rounds 1-2 - result: pass (no remaining findings) - -history: - - timestamp: 2026-07-11 - task: planning - commit: not_required - note: Drafted after resolving tag namespace, legacy cleanup, and --keep decisions. - - timestamp: 2026-07-11 - task: T01-T03 - commit: 6ce7cf3b - note: Implemented isolated tag refs, regression coverage, and PR-only repository guidance.