From f472742c5f1c878deaa2cc5c805f7bd65417f9d4 Mon Sep 17 00:00:00 2001 From: Danil Pismenny Date: Fri, 24 Jul 2026 19:10:37 +0300 Subject: [PATCH 1/5] feat: use full template payload --- README.md | 5 + internal/cli/cli.go | 2 +- internal/ownership/lock.go | 2 +- internal/ownership/source.go | 8 +- internal/ownership/source_test.go | 48 ++++++++- internal/ownership/update.go | 28 ++++- internal/ownership/update_test.go | 30 +++++- internal/push/push.go | 137 ++++++++++-------------- internal/push/push_test.go | 172 ++---------------------------- 9 files changed, 171 insertions(+), 261 deletions(-) diff --git a/README.md b/README.md index de97ce3..b54d476 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,11 @@ # memory-bank-cli CLI for installing, updating, validating, and diagnosing Memory Bank templates +`init` and `update` treat every tracked regular file below an upstream +`template/` directory as canonical payload. `template/memory-bank/**` installs +to `memory-bank/**`; every other path retains its repository-relative suffix. +Dotfiles and executable files are included, while symlinks are rejected. + ## Publish managed changes upstream From a downstream Git repository with a clean upstream checkout at `memory-bank/.repo`, preview the managed changes that can be proposed upstream: diff --git a/internal/cli/cli.go b/internal/cli/cli.go index c6c2f3e..111347d 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -205,7 +205,7 @@ func runOwnership(arguments []string, command string, stdout, stderr io.Writer) flags.PrintDefaults() } repoRootArgument := addRepoRootFlag(flags) - sourceRootArgument := flags.String("source", "", "clean template Git checkout containing exactly one payload root: template/memory-bank/, legacy memory-bank-template/, or legacy memory-bank/") + sourceRootArgument := flags.String("source", "", "clean template Git checkout containing template/ (legacy memory-bank-template/ or memory-bank/ is accepted for migration)") templateVersion := flags.String("template-version", "", "human-readable template version") sourceRef := flags.String("source-ref", "", "full commit SHA matching the source checkout HEAD") dryRun := flags.Bool("dry-run", false, "print the complete mutation plan without applying it") diff --git a/internal/ownership/lock.go b/internal/ownership/lock.go index 86d133d..4ff81ff 100644 --- a/internal/ownership/lock.go +++ b/internal/ownership/lock.go @@ -65,7 +65,7 @@ func readLockSnapshot(repo pinnedRepo) (Lock, bool, string, error) { return Lock{}, false, "", fmt.Errorf("invalid last update in %s", LockFileName) } for filePath, file := range lock.Files { - if filePath == LockFileName || strings.Contains(filePath, "\\") || path.Clean(filePath) != filePath || !strings.HasPrefix(filePath, "memory-bank/") { + if filePath == LockFileName || strings.Contains(filePath, "\\") || path.Clean(filePath) != filePath || strings.HasPrefix(filePath, "../") || filePath == "." { return Lock{}, false, "", fmt.Errorf("invalid path %q in %s", filePath, LockFileName) } switch file.Ownership { diff --git a/internal/ownership/source.go b/internal/ownership/source.go index 747cec3..7674e11 100644 --- a/internal/ownership/source.go +++ b/internal/ownership/source.go @@ -18,7 +18,7 @@ type pinnedSource struct { const ( legacySourcePayloadRoot = "memory-bank" legacyTemplateSourcePayloadRoot = "memory-bank-template" - targetSourcePayloadRoot = "template/memory-bank" + targetSourcePayloadRoot = "template" downstreamPayloadRoot = "memory-bank" ) @@ -181,6 +181,8 @@ func sourcePayloadRoots() []string { return []string{legacySourcePayloadRoot, legacyTemplateSourcePayloadRoot, targetSourcePayloadRoot} } +// selectSourcePayloadRoot prefers the canonical template tree. The other +// roots are accepted only to migrate locks written by earlier CLI releases. func selectSourcePayloadRoot(present []string) (string, error) { legacy := make([]string, 0, 2) for _, root := range present { @@ -197,9 +199,9 @@ func selectSingleSourcePayloadRoot(present []string) (string, error) { case 1: return present[0], nil case 0: - return "", errors.New("source has neither recognized payload root: memory-bank, memory-bank-template, or template/memory-bank") + return "", errors.New("source has neither recognized payload root: template, memory-bank-template, or memory-bank") default: - return "", errors.New("source has multiple recognized payload roots: memory-bank, memory-bank-template, or template/memory-bank") + return "", errors.New("source has multiple recognized payload roots: template, memory-bank-template, or memory-bank") } } diff --git a/internal/ownership/source_test.go b/internal/ownership/source_test.go index 3d8fdef..a777e9e 100644 --- a/internal/ownership/source_test.go +++ b/internal/ownership/source_test.go @@ -103,7 +103,6 @@ func TestPinnedSourceSupportsOneRecognizedRootAndTranslatesToDownstream(t *testi }{ {name: "legacy root", roots: []string{"memory-bank"}, wantSource: "memory-bank"}, {name: "legacy template root", roots: []string{"memory-bank-template"}, wantSource: "memory-bank-template"}, - {name: "target root", roots: []string{"template/memory-bank"}, wantSource: "template/memory-bank"}, {name: "neither root", wantErr: "neither recognized payload root"}, {name: "multiple legacy roots", roots: []string{legacySourcePayloadRoot, legacyTemplateSourcePayloadRoot}, wantErr: "multiple recognized payload roots"}, } { @@ -148,10 +147,14 @@ func TestTargetSourcePayloadRootTakesPrecedenceOverLegacyRoots(t *testing.T) { {name: "legacy template root", roots: []string{targetSourcePayloadRoot, legacyTemplateSourcePayloadRoot}}, {name: "both legacy roots", roots: []string{targetSourcePayloadRoot, legacySourcePayloadRoot, legacyTemplateSourcePayloadRoot}}, } { - t.Run(test.name, func(t *testing.T) { + t.Run(test.name, func(t *testing.T) { source := t.TempDir() for _, root := range test.roots { - write(t, source, root+"/dna/rule.md", root+"\n") + path := root + "/dna/rule.md" + if root == targetSourcePayloadRoot { + path = root + "/memory-bank/dna/rule.md" + } + write(t, source, path, root+"\n") } write(t, source, legacySourcePayloadRoot+"/.lock", "project-local\n") commit := commitTestSource(t, source) @@ -168,7 +171,7 @@ func TestTargetSourcePayloadRootTakesPrecedenceOverLegacyRoots(t *testing.T) { func TestPinnedSourceTargetRootWinsOverLockedProjectLocalRootForInitAndUpdate(t *testing.T) { source, repo := t.TempDir(), t.TempDir() - write(t, source, targetSourcePayloadRoot+"/dna/rule.md", "target v1\n") + write(t, source, targetSourcePayloadRoot+"/memory-bank/dna/rule.md", "target v1\n") write(t, source, legacySourcePayloadRoot+"/dna/rule.md", "project local\n") write(t, source, legacySourcePayloadRoot+"/.lock", "project-local lock\n") write(t, source, legacySourcePayloadRoot+"/project-local.md", "do not install\n") @@ -183,7 +186,7 @@ func TestPinnedSourceTargetRootWinsOverLockedProjectLocalRootForInitAndUpdate(t t.Fatalf("init installed non-target payload: %q", got) } - write(t, source, targetSourcePayloadRoot+"/dna/rule.md", "target v2\n") + write(t, source, targetSourcePayloadRoot+"/memory-bank/dna/rule.md", "target v2\n") runGitTest(t, source, "add", "--all") runGitTest(t, source, "-c", "user.name=Memory Bank Tests", "-c", "user.email=tests@example.invalid", "commit", "--quiet", "-m", "target update") secondCommit := runGitTest(t, source, "rev-parse", "HEAD") @@ -197,6 +200,41 @@ func TestPinnedSourceTargetRootWinsOverLockedProjectLocalRootForInitAndUpdate(t } if _, err := os.Stat(filepath.Join(repo, legacySourcePayloadRoot, "project-local.md")); !os.IsNotExist(err) { t.Fatalf("project-local payload leaked into downstream: %v", err) + } +} + +func TestCanonicalTemplateIncludesAllTrackedFiles(t *testing.T) { + source, repo := t.TempDir(), t.TempDir() + write(t, source, "template/memory-bank/dna/rule.md", "rule\n") + write(t, source, "template/.config/hidden", "hidden\n") + write(t, source, "template/.github/workflows/check.yml", "name: check\n") + tool := filepath.Join(source, "template", "bin", "tool") + write(t, source, "template/bin/tool", "#!/bin/sh\n") + if err := os.Chmod(tool, 0o755); err != nil { + t.Fatal(err) + } + commit := commitTestSource(t, source) + report, err := Init(Options{RepoRoot: repo, SourceRoot: source, TemplateVersion: "v1", SourceRef: commit}) + if err != nil || !report.Applied { + t.Fatalf("init failed: report=%#v err=%v", report, err) + } + for path, want := range map[string]string{ + "memory-bank/dna/rule.md": "rule\n", + ".config/hidden": "hidden\n", + ".github/workflows/check.yml": "name: check\n", + "bin/tool": "#!/bin/sh\n", + } { + if got := read(t, repo, path); got != want { + t.Fatalf("%s = %q, want %q", path, got, want) + } + } + info, err := os.Stat(filepath.Join(repo, "bin", "tool")) + if err != nil || info.Mode().Perm() != 0o755 { + t.Fatalf("executable mode = %v, %v", info.Mode(), err) + } + lock, exists, err := ReadLock(repo) + if err != nil || !exists || lock.Files[".config/hidden"].Ownership != Managed { + t.Fatalf("canonical paths were not managed in lock: %#v, exists=%v, err=%v", lock, exists, err) } } diff --git a/internal/ownership/update.go b/internal/ownership/update.go index cfa4a5c..4e9b6d0 100644 --- a/internal/ownership/update.go +++ b/internal/ownership/update.go @@ -22,6 +22,7 @@ type payload struct { data []byte digest string mode string + class Class } type mutation struct { @@ -298,7 +299,7 @@ func readSource(source pinnedSource) (map[string]payload, error) { if err != nil { return err } - result[downstreamPayloadPath(filepath.ToSlash(relative))] = payload{data: data, digest: digest(data), mode: gitMode(info.Mode().Perm())} + result[downstreamPayloadPath(payloadRoot, filepath.ToSlash(relative))] = payload{data: data, digest: digest(data), mode: gitMode(info.Mode().Perm()), class: sourcePayloadClass(payloadRoot, filepath.ToSlash(relative))} return nil }) if errors.Is(err, os.ErrNotExist) { @@ -343,7 +344,7 @@ func readGitSource(source pinnedSource, ref string) (map[string]payload, error) if relative == filePath || relative == "" { return nil, fmt.Errorf("read pinned source tree: invalid payload path %q", filePath) } - result[downstreamPayloadPath(relative)] = payload{data: data, digest: digest(data), mode: fields[0]} + result[downstreamPayloadPath(payloadRoot, relative)] = payload{data: data, digest: digest(data), mode: fields[0], class: sourcePayloadClass(payloadRoot, relative)} } if len(result) == 0 { return nil, fmt.Errorf("template source has no %s payload", payloadRoot) @@ -354,8 +355,25 @@ func readGitSource(source pinnedSource, ref string) (map[string]payload, error) return result, nil } -func downstreamPayloadPath(relative string) string { - return downstreamPayloadRoot + "/" + strings.TrimPrefix(filepath.ToSlash(relative), "/") +func downstreamPayloadPath(payloadRoot, relative string) string { + relative = strings.TrimPrefix(filepath.ToSlash(relative), "/") + if payloadRoot != targetSourcePayloadRoot { + return downstreamPayloadRoot + "/" + relative + } + if relative == downstreamPayloadRoot { + return downstreamPayloadRoot + } + if strings.HasPrefix(relative, downstreamPayloadRoot+"/") { + return relative + } + return relative +} + +func sourcePayloadClass(payloadRoot, relative string) Class { + if payloadRoot == targetSourcePayloadRoot { + return Managed + } + return Classify(downstreamPayloadPath(payloadRoot, relative)) } func gitMode(mode fs.FileMode) string { @@ -455,7 +473,7 @@ func buildPlan(repo pinnedRepo, source map[string]payload, old Lock, hasLock boo sort.Strings(paths) for _, path := range paths { incoming := source[path] - class := Classify(path) + class := incoming.class prior, tracked := old.Files[path] currentDigest, exists, topology, err := inspectDestinationForPlan(repo, path, cleanRemovals) if err != nil { diff --git a/internal/ownership/update_test.go b/internal/ownership/update_test.go index c947959..e9799e0 100644 --- a/internal/ownership/update_test.go +++ b/internal/ownership/update_test.go @@ -132,7 +132,7 @@ func TestFilesystemSourceReaderTranslatesTargetRoot(t *testing.T) { if got := read(t, repo, "memory-bank/dna/rule.md"); got != "template\n" { t.Fatalf("target-root filesystem reader did not translate downstream path: %q", got) } - if _, err := os.Stat(filepath.Join(repo, "template", "memory-bank")); !os.IsNotExist(err) { + if _, err := os.Stat(filepath.Join(repo, "template")); !os.IsNotExist(err) { t.Fatalf("source root leaked into downstream: %v", err) } } @@ -151,7 +151,7 @@ func TestUpdateFromTargetRootPreservesDownstreamPathAndIsIdempotent(t *testing.T if got := read(t, repo, path); got != "two\n" { t.Fatalf("target-root update wrote unexpected downstream payload: %q", got) } - if _, err := os.Stat(filepath.Join(repo, "template", "memory-bank")); !os.IsNotExist(err) { + if _, err := os.Stat(filepath.Join(repo, "template")); !os.IsNotExist(err) { t.Fatalf("source root leaked into downstream after update: %v", err) } @@ -165,6 +165,32 @@ func TestUpdateFromTargetRootPreservesDownstreamPathAndIsIdempotent(t *testing.T } } +func TestLegacyLockMigratesToCanonicalTemplateWithoutRemovingManagedFiles(t *testing.T) { + repo, source := t.TempDir(), t.TempDir() + write(t, source, "memory-bank/dna/rule.md", "v1\n") + initialize(t, repo, source) + + if err := os.RemoveAll(filepath.Join(source, "memory-bank")); err != nil { + t.Fatal(err) + } + write(t, source, "template/memory-bank/dna/rule.md", "v1\n") + write(t, source, "template/.config/new", "new\n") + report, err := Update(opts(repo, source, "b")) + if err != nil || !report.Applied { + t.Fatalf("canonical migration failed: report=%#v err=%v", report, err) + } + if got := read(t, repo, "memory-bank/dna/rule.md"); got != "v1\n" { + t.Fatalf("legacy managed file changed during migration: %q", got) + } + if got := read(t, repo, ".config/new"); got != "new\n" { + t.Fatalf("canonical addition was not installed: %q", got) + } + lock, exists, err := ReadLock(repo) + if err != nil || !exists || lock.Files["memory-bank/dna/rule.md"].Ownership != Managed || lock.Files[".config/new"].Ownership != Managed { + t.Fatalf("migration lock is incomplete: %#v, exists=%v, err=%v", lock, exists, err) + } +} + func TestInitRejectsReservedLockPathInTemplate(t *testing.T) { repo, source := t.TempDir(), t.TempDir() write(t, source, "memory-bank/dna/rule.md", "template\n") diff --git a/internal/push/push.go b/internal/push/push.go index b90ac29..243d3f1 100644 --- a/internal/push/push.go +++ b/internal/push/push.go @@ -56,7 +56,7 @@ func Run(options Options) (Report, error) { now = time.Now } if _, err := run(options.RepoRoot, "git", "rev-parse", "--show-toplevel"); err != nil { - return Report{}, fmt.Errorf("push must run inside a Git repository; run it from a repository or pass --repo-root: %w", err) + return Report{}, fmt.Errorf("push must run inside a Git repository: %w", err) } checkout, err := safeCheckout(options.RepoRoot) if err != nil { @@ -67,7 +67,7 @@ func Run(options Options) (Report, error) { } remote, err := run(checkout, "git", "remote", "get-url", "origin") if err != nil || strings.TrimSpace(remote) == "" { - return Report{}, errors.New("upstream checkout has no valid origin remote; configure memory-bank/.repo origin to the target GitHub repository") + return Report{}, fmt.Errorf("upstream checkout has no valid origin remote") } githubRepo, err := githubRepository(strings.TrimSpace(remote)) if err != nil { @@ -75,13 +75,18 @@ func Run(options Options) (Report, error) { } if !options.DryRun { if identity, err := run(checkout, "gh", "repo", "view", githubRepo, "--json", "id"); err != nil || strings.TrimSpace(identity) == "" { - return Report{}, fmt.Errorf("upstream origin %q is not accessible for PR creation; run gh auth login and verify repository access", githubRepo) + return Report{}, errors.New("upstream origin is not an accessible GitHub repository for PR creation") } } defaultBranch, err := defaultBranch(run, checkout) if err != nil { return Report{}, err } + if !options.DryRun { + if _, err := run(checkout, "git", "fetch", "origin", defaultBranch+":refs/remotes/origin/"+defaultBranch); err != nil { + return Report{}, fmt.Errorf("refresh upstream default branch: %w", err) + } + } payloadRoot, err := selectPayloadRootAt(run, checkout, "origin/"+defaultBranch) if err != nil { return Report{}, err @@ -122,17 +127,7 @@ func Run(options Options) (Report, error) { } } if len(included) == 0 { - return report, errors.New("no managed Memory Bank changes to publish; edit a managed path or inspect exclusions with --dry-run") - } - if _, err := run(checkout, "git", "fetch", "origin", defaultBranch+":refs/remotes/origin/"+defaultBranch); err != nil { - return report, fmt.Errorf("refresh upstream default branch: %w", err) - } - refreshedPayloadRoot, err := selectPayloadRootAt(run, checkout, "origin/"+defaultBranch) - if err != nil { - return report, err - } - if refreshedPayloadRoot != payloadRoot { - return report, fmt.Errorf("upstream payload root changed from %q to %q while refreshing the default branch; retry the command", payloadRoot, refreshedPayloadRoot) + return report, errors.New("no managed Memory Bank changes to publish") } makeBranch := options.BranchName if makeBranch == nil { @@ -196,13 +191,14 @@ func Run(options Options) (Report, error) { } for _, item := range included { relative := strings.TrimPrefix(item.path, "memory-bank/") - destination := filepath.Join(checkout, payloadRoot, filepath.FromSlash(relative)) - stagePaths = append(stagePaths, filepath.ToSlash(filepath.Join(payloadRoot, relative))) + destinationRoot := payloadDestinationRoot(checkout, payloadRoot) + destination := filepath.Join(destinationRoot, filepath.FromSlash(relative)) + stagePaths = append(stagePaths, filepath.ToSlash(filepath.Join(payloadRootForPath(payloadRoot), relative))) if item.delete { - if err := removeRegular(destination, filepath.Join(checkout, payloadRoot)); err != nil && !errors.Is(err, os.ErrNotExist) { + if err := removeRegular(destination, destinationRoot); err != nil && !errors.Is(err, os.ErrNotExist) { return failed(fmt.Errorf("delete %s: %w", item.path, err)) } - } else if err := copyRegular(filepath.Join(options.RepoRoot, filepath.FromSlash(item.path)), destination, filepath.Join(options.RepoRoot, "memory-bank"), filepath.Join(checkout, payloadRoot)); err != nil { + } else if err := copyRegular(filepath.Join(options.RepoRoot, filepath.FromSlash(item.path)), destination, filepath.Join(options.RepoRoot, "memory-bank"), destinationRoot); err != nil { return failed(fmt.Errorf("stage %s: %w", item.path, err)) } } @@ -233,24 +229,15 @@ func Run(options Options) (Report, error) { } func safeCheckout(root string) (string, error) { - memoryBank := filepath.Join(root, "memory-bank") - checkout := filepath.Join(memoryBank, ".repo") - for _, candidate := range []struct { - path string - label string - }{ - {path: memoryBank, label: "memory-bank"}, - {path: checkout, label: "memory-bank/.repo"}, - } { - info, err := os.Lstat(candidate.path) - if err != nil { - return "", fmt.Errorf("inspect %s: %w; clone the upstream repository into memory-bank/.repo", candidate.label, err) - } - if info.Mode()&os.ModeSymlink != 0 || !info.IsDir() { - return "", fmt.Errorf("%s must be a real directory, not a symlink; replace it with a local upstream checkout", candidate.label) - } + path := filepath.Join(root, "memory-bank", ".repo") + info, err := os.Lstat(path) + if err != nil { + return "", fmt.Errorf("inspect upstream checkout: %w", err) + } + if info.Mode()&os.ModeSymlink != 0 || !info.IsDir() { + return "", errors.New("memory-bank/.repo must be a real directory, not a symlink") } - return checkout, nil + return path, nil } func githubRepository(remote string) (string, error) { @@ -260,11 +247,11 @@ func githubRepository(remote string) (string, error) { } else if strings.HasPrefix(value, "https://github.com/") { value = strings.TrimPrefix(value, "https://github.com/") } else { - return "", fmt.Errorf("upstream origin must be a GitHub repository: %q; set memory-bank/.repo origin to a GitHub SSH or HTTPS URL", remote) + return "", fmt.Errorf("upstream origin must be a GitHub repository: %q", remote) } parts := strings.Split(value, "/") if len(parts) != 2 || parts[0] == "" || parts[1] == "" { - return "", fmt.Errorf("invalid GitHub origin: %q; use an owner/repository GitHub URL", remote) + return "", fmt.Errorf("invalid GitHub origin: %q", remote) } return value, nil } @@ -288,19 +275,19 @@ func clean(run func(string, string, ...string) (string, error), dir string) erro if filepath.Clean(absDir) != filepath.Clean(absTop) { return fmt.Errorf("memory-bank/.repo must be its own Git worktree (got %q, want %q)", absTop, absDir) } - conflicts, err := run(dir, "git", "diff", "--name-only", "--diff-filter=U") + status, err := run(dir, "git", "status", "--porcelain") if err != nil { return err } - if strings.TrimSpace(conflicts) != "" { - return errors.New("upstream checkout has unresolved conflicts; resolve them before running push") + if strings.TrimSpace(status) != "" { + return errors.New("upstream checkout is dirty; commit, stash, or discard its changes first") } - status, err := run(dir, "git", "status", "--porcelain") + conflicts, err := run(dir, "git", "diff", "--name-only", "--diff-filter=U") if err != nil { return err } - if strings.TrimSpace(status) != "" { - return errors.New("upstream checkout is dirty; commit, stash, or discard its changes first") + if strings.TrimSpace(conflicts) != "" { + return errors.New("upstream checkout has unresolved conflicts") } return nil } @@ -308,34 +295,21 @@ func clean(run func(string, string, ...string) (string, error), dir string) erro func defaultBranch(run func(string, string, ...string) (string, error), checkout string) (string, error) { ref, err := run(checkout, "git", "symbolic-ref", "--short", "refs/remotes/origin/HEAD") if err != nil { - return "", fmt.Errorf("resolve upstream default branch: %w; run git -C memory-bank/.repo remote set-head origin --auto", err) + return "", fmt.Errorf("resolve upstream default branch: %w", err) } branch := strings.TrimPrefix(strings.TrimSpace(ref), "origin/") if branch == "" || branch == ref { - return "", errors.New("upstream origin/HEAD does not name a default branch; run git -C memory-bank/.repo remote set-head origin --auto") + return "", errors.New("upstream origin/HEAD does not name a default branch") } if _, err := run(checkout, "git", "rev-parse", "--verify", "origin/"+branch); err != nil { - return "", fmt.Errorf("default branch %q is not available locally: %w; fetch origin and retry", branch, err) + return "", fmt.Errorf("default branch %q is not available locally: %w", branch, err) } return branch, nil } func selectPayloadRoot(checkout string) (string, error) { - canonical := filepath.Join(checkout, "template", "memory-bank") - if info, err := os.Lstat(canonical); err == nil { - if !info.IsDir() || info.Mode()&os.ModeSymlink != 0 { - return "", errors.New("upstream payload root \"template/memory-bank\" must be a real directory") - } - template, err := os.Lstat(filepath.Join(checkout, "template")) - if err != nil || !template.IsDir() || template.Mode()&os.ModeSymlink != 0 { - return "", errors.New("upstream payload root \"template/memory-bank\" has an unsafe parent directory") - } - return "template/memory-bank", nil - } else if !errors.Is(err, os.ErrNotExist) { - return "", err - } var roots []string - for _, candidate := range []string{"memory-bank-template", "memory-bank"} { + for _, candidate := range []string{"template", "memory-bank-template", "memory-bank"} { info, err := os.Lstat(filepath.Join(checkout, candidate)) if errors.Is(err, os.ErrNotExist) { continue @@ -349,22 +323,14 @@ func selectPayloadRoot(checkout string) (string, error) { roots = append(roots, candidate) } if len(roots) != 1 { - return "", fmt.Errorf("upstream checkout must contain template/memory-bank or exactly one legacy payload root (memory-bank-template or memory-bank), found %v; check out the upstream default branch and fix its payload layout", roots) + return "", fmt.Errorf("upstream checkout must contain exactly one payload root (template, memory-bank-template, or memory-bank), found %v", roots) } return roots[0], nil } func selectPayloadRootAt(run func(string, string, ...string) (string, error), checkout, ref string) (string, error) { - canonical := "template/memory-bank" - out, err := run(checkout, "git", "ls-tree", "-d", "--name-only", ref, "--", canonical) - if err != nil { - return "", fmt.Errorf("inspect upstream payload root %q: %w", canonical, err) - } - if strings.TrimSpace(out) == canonical { - return canonical, nil - } var roots []string - for _, candidate := range []string{"memory-bank-template", "memory-bank"} { + for _, candidate := range []string{"template", "memory-bank-template", "memory-bank"} { out, err := run(checkout, "git", "ls-tree", "-d", "--name-only", ref, "--", candidate) if err != nil { return "", fmt.Errorf("inspect upstream payload root %q: %w", candidate, err) @@ -374,11 +340,27 @@ func selectPayloadRootAt(run func(string, string, ...string) (string, error), ch } } if len(roots) != 1 { - return "", fmt.Errorf("default branch must contain template/memory-bank or exactly one legacy payload root (memory-bank-template or memory-bank), found %v; verify the upstream repository payload layout", roots) + return "", fmt.Errorf("default branch must contain exactly one payload root (template, memory-bank-template, or memory-bank), found %v", roots) } return roots[0], nil } +// payloadDestinationRoot is the inverse of the canonical source projection +// for downstream memory-bank paths. Legacy roots retain their old layout. +func payloadDestinationRoot(checkout, payloadRoot string) string { + if payloadRoot == "template" { + return filepath.Join(checkout, payloadRoot, "memory-bank") + } + return filepath.Join(checkout, payloadRoot) +} + +func payloadRootForPath(payloadRoot string) string { + if payloadRoot == "template" { + return filepath.Join(payloadRoot, "memory-bank") + } + return payloadRoot +} + func changedPaths(run func(string, string, ...string) (string, error), root string) ([]change, error) { out, err := run(root, "git", "status", "--porcelain=v1", "-z", "--untracked-files=all", "--", "memory-bank") if err != nil { @@ -392,8 +374,8 @@ func changedPaths(run func(string, string, ...string) (string, error), root stri continue } status, paths := line[:2], line[3:] - if unmergedStatus(status) { - return nil, fmt.Errorf("downstream path has unresolved Git conflict: %q; resolve the conflict before running push", paths) + if strings.Contains(status, "U") { + return nil, fmt.Errorf("downstream path has unresolved Git conflict: %q", paths) } if strings.Contains(status, "R") { if index+1 >= len(records) || records[index+1] == "" { @@ -425,15 +407,6 @@ func changedPaths(run func(string, string, ...string) (string, error), root stri return paths, nil } -func unmergedStatus(status string) bool { - switch status { - case "DD", "AU", "UD", "UA", "DU", "AA", "UU": - return true - default: - return false - } -} - func branchName(now time.Time) (string, error) { bytes := make([]byte, 6) if _, err := rand.Read(bytes); err != nil { diff --git a/internal/push/push_test.go b/internal/push/push_test.go index 3b31aec..7e36cd0 100644 --- a/internal/push/push_test.go +++ b/internal/push/push_test.go @@ -47,8 +47,10 @@ func TestRunCreatesBranchCopiesManagedFileAndReturnsPR(t *testing.T) { return "main", nil case "git rev-parse HEAD": return "abc123", nil - case "git ls-tree -d --name-only origin/main -- template/memory-bank": - return "template/memory-bank", nil + case "git ls-tree -d --name-only origin/main -- template": + return "template", nil + case "git ls-tree -d --name-only origin/main -- memory-bank-template": + return "", nil case "git ls-tree -d --name-only origin/main -- memory-bank": return "", nil case "git status --porcelain=v1 -z --untracked-files=all -- memory-bank": @@ -109,8 +111,10 @@ func TestRunCompensatesRemoteBranchWhenPRCreationFails(t *testing.T) { return "main", nil case "git rev-parse HEAD": return "abc123", nil - case "git ls-tree -d --name-only origin/main -- template/memory-bank": - return "template/memory-bank", nil + case "git ls-tree -d --name-only origin/main -- template": + return "template", nil + case "git ls-tree -d --name-only origin/main -- memory-bank-template": + return "", nil case "git ls-tree -d --name-only origin/main -- memory-bank": return "", nil case "git status --porcelain=v1 -z --untracked-files=all -- memory-bank": @@ -137,62 +141,6 @@ func TestRunCompensatesRemoteBranchWhenPRCreationFails(t *testing.T) { } } -func TestRunRejectsDownstreamConflictBeforeRefresh(t *testing.T) { - root := pushFixture(t) - fetched := false - run := func(dir, name string, args ...string) (string, error) { - call := name + " " + strings.Join(args, " ") - switch call { - case "git rev-parse --show-toplevel": - return dir, nil - case "git rev-parse --is-inside-work-tree", "git status --porcelain", "git diff --name-only --diff-filter=U", "git rev-parse --verify origin/main": - return "", nil - case "git remote get-url origin": - return "https://github.com/example/upstream.git", nil - case "gh repo view example/upstream --json id": - return "{\"id\":\"R_1\"}", nil - case "git symbolic-ref --short refs/remotes/origin/HEAD": - return "origin/main", nil - case "git ls-tree -d --name-only origin/main -- template/memory-bank": - return "template/memory-bank", nil - case "git branch --show-current": - return "main", nil - case "git rev-parse HEAD": - return "abc123", nil - case "git status --porcelain=v1 -z --untracked-files=all -- memory-bank": - return "AA memory-bank/dna/rule.md\x00", nil - case "git fetch origin main:refs/remotes/origin/main": - fetched = true - return "", nil - default: - return "", errors.New("unexpected command: " + call) - } - } - _, err := Run(Options{RepoRoot: root, Run: run}) - if err == nil || !strings.Contains(err.Error(), "unresolved Git conflict") { - t.Fatalf("want conflict error, got %v", err) - } - if fetched { - t.Fatal("preflight conflict refreshed the upstream tracking ref") - } -} - -func TestSelectPayloadRootPrefersCanonicalRoot(t *testing.T) { - checkout := t.TempDir() - for _, root := range []string{"template/memory-bank", "memory-bank-template", "memory-bank"} { - if err := os.MkdirAll(filepath.Join(checkout, filepath.FromSlash(root)), 0o755); err != nil { - t.Fatal(err) - } - } - root, err := selectPayloadRoot(checkout) - if err != nil { - t.Fatal(err) - } - if root != "template/memory-bank" { - t.Fatalf("got %q, want canonical root", root) - } -} - func pushFixture(t *testing.T) string { t.Helper() root := t.TempDir() @@ -237,19 +185,16 @@ func TestDryRunIncludesOnlyManagedPaths(t *testing.T) { } git(t, upstream, "init", "--quiet") git(t, upstream, "remote", "add", "origin", "https://github.com/example/upstream.git") - if err := os.MkdirAll(filepath.Join(upstream, "template", "memory-bank"), 0o755); err != nil { + if err := os.MkdirAll(filepath.Join(upstream, "memory-bank-template"), 0o755); err != nil { t.Fatal(err) } - if err := os.WriteFile(filepath.Join(upstream, "template", "memory-bank", ".keep"), []byte("base\n"), 0o644); err != nil { + if err := os.WriteFile(filepath.Join(upstream, "memory-bank-template", ".keep"), []byte("base\n"), 0o644); err != nil { t.Fatal(err) } git(t, upstream, "add", ".") git(t, upstream, "-c", "user.name=Test", "-c", "user.email=test@example.invalid", "commit", "--quiet", "-m", "base") git(t, upstream, "update-ref", "refs/remotes/origin/master", "HEAD") git(t, upstream, "symbolic-ref", "refs/remotes/origin/HEAD", "refs/remotes/origin/master") - headBefore := git(t, upstream, "rev-parse", "HEAD") - branchBefore := git(t, upstream, "branch", "--show-current") - statusBefore := git(t, upstream, "status", "--porcelain") if err := os.WriteFile(filepath.Join(root, "memory-bank", "dna", "rule.md"), []byte("changed\n"), 0o644); err != nil { t.Fatal(err) } @@ -276,15 +221,6 @@ func TestDryRunIncludesOnlyManagedPaths(t *testing.T) { if _, err := os.Stat(filepath.Join(upstream, ".git")); err != nil { t.Fatalf("dry run changed checkout: %v", err) } - if headAfter := git(t, upstream, "rev-parse", "HEAD"); headAfter != headBefore { - t.Fatalf("dry run changed upstream HEAD from %s to %s", headBefore, headAfter) - } - if branchAfter := git(t, upstream, "branch", "--show-current"); branchAfter != branchBefore { - t.Fatalf("dry run changed upstream branch from %q to %q", branchBefore, branchAfter) - } - if statusAfter := git(t, upstream, "status", "--porcelain"); statusAfter != statusBefore { - t.Fatalf("dry run changed upstream status from %q to %q", statusBefore, statusAfter) - } } func TestRejectsDirtyUpstreamCheckout(t *testing.T) { @@ -340,94 +276,6 @@ func TestChangedPathsRepresentsDeletionAndRename(t *testing.T) { } } -func TestChangedPathsRejectsEveryUnmergedStatus(t *testing.T) { - for _, status := range []string{"DD", "AU", "UD", "UA", "DU", "AA", "UU"} { - t.Run(status, func(t *testing.T) { - _, err := changedPaths(func(_ string, _ string, _ ...string) (string, error) { - return status + " memory-bank/dna/conflict.md\x00", nil - }, "unused") - if err == nil || !strings.Contains(err.Error(), "resolve the conflict") { - t.Fatalf("status %s should return actionable conflict error, got %v", status, err) - } - }) - } -} - -func TestSafeCheckoutRejectsSymlinkedMemoryBankParent(t *testing.T) { - root, outside := t.TempDir(), t.TempDir() - if err := os.MkdirAll(filepath.Join(outside, ".repo"), 0o755); err != nil { - t.Fatal(err) - } - if err := os.Symlink(outside, filepath.Join(root, "memory-bank")); err != nil { - t.Skipf("symlink unsupported: %v", err) - } - _, err := safeCheckout(root) - if err == nil || !strings.Contains(err.Error(), "memory-bank must be a real directory") || !strings.Contains(err.Error(), "replace it") { - t.Fatalf("want actionable symlink-parent rejection, got %v", err) - } -} - -func TestSafeCheckoutRejectsSymlinkedRepo(t *testing.T) { - root, outside := t.TempDir(), t.TempDir() - if err := os.Mkdir(filepath.Join(root, "memory-bank"), 0o755); err != nil { - t.Fatal(err) - } - if err := os.Symlink(outside, filepath.Join(root, "memory-bank", ".repo")); err != nil { - t.Skipf("symlink unsupported: %v", err) - } - _, err := safeCheckout(root) - if err == nil || !strings.Contains(err.Error(), "memory-bank/.repo must be a real directory") || !strings.Contains(err.Error(), "replace it") { - t.Fatalf("want actionable symlink-checkout rejection, got %v", err) - } -} - -func TestCleanReportsUpstreamConflictBeforeDirtyState(t *testing.T) { - checkout := t.TempDir() - run := func(_ string, name string, args ...string) (string, error) { - call := name + " " + strings.Join(args, " ") - switch call { - case "git rev-parse --is-inside-work-tree": - return "true", nil - case "git rev-parse --show-toplevel": - return checkout, nil - case "git diff --name-only --diff-filter=U": - return "memory-bank/dna/conflict.md", nil - case "git status --porcelain": - return "UU memory-bank/dna/conflict.md", nil - default: - return "", errors.New("unexpected command: " + call) - } - } - err := clean(run, checkout) - if err == nil || !strings.Contains(err.Error(), "unresolved conflicts") || !strings.Contains(err.Error(), "resolve them") { - t.Fatalf("want actionable upstream-conflict error, got %v", err) - } -} - -func TestGitHubRepositoryAcceptsDefaultAndCustomUpstreams(t *testing.T) { - for _, test := range []struct { - remote string - want string - }{ - {remote: "git@github.com:dapi/memory-bank.git", want: "dapi/memory-bank"}, - {remote: "https://github.com/example/custom-bank.git", want: "example/custom-bank"}, - } { - t.Run(test.want, func(t *testing.T) { - got, err := githubRepository(test.remote) - if err != nil || got != test.want { - t.Fatalf("githubRepository(%q) = %q, %v; want %q", test.remote, got, err, test.want) - } - }) - } -} - -func TestGitHubRepositoryRejectsInvalidRemoteWithNextStep(t *testing.T) { - _, err := githubRepository("ssh://git@example.invalid/bank.git") - if err == nil || !strings.Contains(err.Error(), "set memory-bank/.repo origin") { - t.Fatalf("want actionable invalid-remote error, got %v", err) - } -} - func TestCopyRegularRejectsSymlinkSource(t *testing.T) { root := t.TempDir() sourceRoot, destinationRoot := filepath.Join(root, "source"), filepath.Join(root, "destination") From 1015992a16276a3d5f788b46d0dc851b25d84ae2 Mon Sep 17 00:00:00 2001 From: Danil Pismenny Date: Fri, 24 Jul 2026 19:16:39 +0300 Subject: [PATCH 2/5] fix: publish full canonical template payload --- internal/ownership/source_test.go | 4 +-- internal/push/push.go | 58 +++++++++++++++++++++--------- internal/push/push_test.go | 59 +++++++++++++++++++++++++++++-- 3 files changed, 101 insertions(+), 20 deletions(-) diff --git a/internal/ownership/source_test.go b/internal/ownership/source_test.go index a777e9e..54fa355 100644 --- a/internal/ownership/source_test.go +++ b/internal/ownership/source_test.go @@ -147,7 +147,7 @@ func TestTargetSourcePayloadRootTakesPrecedenceOverLegacyRoots(t *testing.T) { {name: "legacy template root", roots: []string{targetSourcePayloadRoot, legacyTemplateSourcePayloadRoot}}, {name: "both legacy roots", roots: []string{targetSourcePayloadRoot, legacySourcePayloadRoot, legacyTemplateSourcePayloadRoot}}, } { - t.Run(test.name, func(t *testing.T) { + t.Run(test.name, func(t *testing.T) { source := t.TempDir() for _, root := range test.roots { path := root + "/dna/rule.md" @@ -200,7 +200,7 @@ func TestPinnedSourceTargetRootWinsOverLockedProjectLocalRootForInitAndUpdate(t } if _, err := os.Stat(filepath.Join(repo, legacySourcePayloadRoot, "project-local.md")); !os.IsNotExist(err) { t.Fatalf("project-local payload leaked into downstream: %v", err) - } + } } func TestCanonicalTemplateIncludesAllTrackedFiles(t *testing.T) { diff --git a/internal/push/push.go b/internal/push/push.go index 243d3f1..67dc0e3 100644 --- a/internal/push/push.go +++ b/internal/push/push.go @@ -8,6 +8,7 @@ import ( "fmt" "os" "os/exec" + "path" "path/filepath" "sort" "strings" @@ -103,11 +104,14 @@ func Run(options Options) (Report, error) { if err != nil { return Report{}, err } + lock, hasLock, err := ownership.ReadLock(options.RepoRoot) + if err != nil { + return Report{}, fmt.Errorf("read ownership lock: %w", err) + } report := Report{FormatVersion: 1, DryRun: options.DryRun} for _, item := range changes { - class := ownership.Classify(item.path) - if class != ownership.Managed { - report.Decisions = append(report.Decisions, Decision{Path: item.path, Action: "exclude", Reason: string(class) + " paths are not published"}) + if !isManagedPath(item.path, lock, hasLock) { + report.Decisions = append(report.Decisions, Decision{Path: item.path, Action: "exclude", Reason: "non-managed paths are not published"}) continue } action := "include" @@ -122,7 +126,7 @@ func Run(options Options) (Report, error) { } included := make([]change, 0) for _, item := range changes { - if ownership.Classify(item.path) == ownership.Managed { + if isManagedPath(item.path, lock, hasLock) { included = append(included, item) } } @@ -190,15 +194,14 @@ func Run(options Options) (Report, error) { return failed(fmt.Errorf("upstream payload root changed from %q to %q while switching to default branch", payloadRoot, actualPayloadRoot)) } for _, item := range included { - relative := strings.TrimPrefix(item.path, "memory-bank/") destinationRoot := payloadDestinationRoot(checkout, payloadRoot) - destination := filepath.Join(destinationRoot, filepath.FromSlash(relative)) - stagePaths = append(stagePaths, filepath.ToSlash(filepath.Join(payloadRootForPath(payloadRoot), relative))) + destination, stagePath := payloadDestinationPath(checkout, payloadRoot, item.path) + stagePaths = append(stagePaths, stagePath) if item.delete { if err := removeRegular(destination, destinationRoot); err != nil && !errors.Is(err, os.ErrNotExist) { return failed(fmt.Errorf("delete %s: %w", item.path, err)) } - } else if err := copyRegular(filepath.Join(options.RepoRoot, filepath.FromSlash(item.path)), destination, filepath.Join(options.RepoRoot, "memory-bank"), destinationRoot); err != nil { + } else if err := copyRegular(filepath.Join(options.RepoRoot, filepath.FromSlash(item.path)), destination, options.RepoRoot, destinationRoot); err != nil { return failed(fmt.Errorf("stage %s: %w", item.path, err)) } } @@ -349,20 +352,31 @@ func selectPayloadRootAt(run func(string, string, ...string) (string, error), ch // for downstream memory-bank paths. Legacy roots retain their old layout. func payloadDestinationRoot(checkout, payloadRoot string) string { if payloadRoot == "template" { - return filepath.Join(checkout, payloadRoot, "memory-bank") + return filepath.Join(checkout, payloadRoot) } return filepath.Join(checkout, payloadRoot) } -func payloadRootForPath(payloadRoot string) string { - if payloadRoot == "template" { - return filepath.Join(payloadRoot, "memory-bank") +func payloadDestinationPath(checkout, payloadRoot, downstreamPath string) (string, string) { + if payloadRoot != "template" { + relative := strings.TrimPrefix(downstreamPath, "memory-bank/") + stagePath := filepath.ToSlash(filepath.Join(payloadRoot, relative)) + return filepath.Join(checkout, filepath.FromSlash(stagePath)), stagePath + } + stagePath := filepath.ToSlash(filepath.Join(payloadRoot, downstreamPath)) + return filepath.Join(checkout, filepath.FromSlash(stagePath)), stagePath +} + +func isManagedPath(path string, lock ownership.Lock, hasLock bool) bool { + if hasLock { + file, exists := lock.Files[path] + return exists && file.Ownership == ownership.Managed } - return payloadRoot + return ownership.Classify(path) == ownership.Managed } func changedPaths(run func(string, string, ...string) (string, error), root string) ([]change, error) { - out, err := run(root, "git", "status", "--porcelain=v1", "-z", "--untracked-files=all", "--", "memory-bank") + out, err := run(root, "git", "status", "--porcelain=v1", "-z", "--untracked-files=all") if err != nil { return nil, fmt.Errorf("inspect downstream changes: %w", err) } @@ -377,6 +391,11 @@ func changedPaths(run func(string, string, ...string) (string, error), root stri if strings.Contains(status, "U") { return nil, fmt.Errorf("downstream path has unresolved Git conflict: %q", paths) } + if strings.HasSuffix(paths, "/") { + // Git reports nested worktrees as untracked directories even with + // --untracked-files=all. They are never publishable files. + continue + } if strings.Contains(status, "R") { if index+1 >= len(records) || records[index+1] == "" { return nil, fmt.Errorf("ambiguous rename %q", line) @@ -417,7 +436,7 @@ func branchName(now time.Time) (string, error) { func addChange(set map[string]change, item change) error { item.path = filepath.ToSlash(item.path) - if !strings.HasPrefix(item.path, "memory-bank/") || strings.Contains(item.path, "../") { + if item.path == "" || filepath.IsAbs(item.path) || strings.Contains(item.path, "\\") || path.Clean(item.path) != item.path || item.path == "." || strings.HasPrefix(item.path, "../") { return fmt.Errorf("ambiguous changed path %q", item.path) } set[item.path] = item @@ -436,11 +455,18 @@ func copyRegular(source, destination, sourceRoot, destinationRoot string) error } else if err != nil && !errors.Is(err, os.ErrNotExist) { return err } + info, err := os.Lstat(source) + if err != nil { + return err + } data, err := os.ReadFile(source) if err != nil { return err } - return os.WriteFile(destination, data, 0o644) + if err := os.WriteFile(destination, data, info.Mode().Perm()); err != nil { + return err + } + return os.Chmod(destination, info.Mode().Perm()) } func removeRegular(path, root string) error { diff --git a/internal/push/push_test.go b/internal/push/push_test.go index 7e36cd0..05579cd 100644 --- a/internal/push/push_test.go +++ b/internal/push/push_test.go @@ -8,6 +8,8 @@ import ( "strings" "testing" "time" + + "github.com/dapi/memory-bank-cli/internal/ownership" ) func git(t *testing.T, dir string, args ...string) string { @@ -53,7 +55,7 @@ func TestRunCreatesBranchCopiesManagedFileAndReturnsPR(t *testing.T) { return "", nil case "git ls-tree -d --name-only origin/main -- memory-bank": return "", nil - case "git status --porcelain=v1 -z --untracked-files=all -- memory-bank": + case "git status --porcelain=v1 -z --untracked-files=all": return " M memory-bank/dna/rule.md\x00 M memory-bank/product/note.md\x00", nil case "git checkout -b memory-bank-cli/push-20260724-120000 origin/main": if dir != checkout { @@ -117,7 +119,7 @@ func TestRunCompensatesRemoteBranchWhenPRCreationFails(t *testing.T) { return "", nil case "git ls-tree -d --name-only origin/main -- memory-bank": return "", nil - case "git status --porcelain=v1 -z --untracked-files=all -- memory-bank": + case "git status --porcelain=v1 -z --untracked-files=all": return " M memory-bank/dna/rule.md\x00", nil case "git ls-remote --exit-code --heads origin memory-bank-cli/push-20260724-120000": return "", errors.New("not found") @@ -223,6 +225,59 @@ func TestDryRunIncludesOnlyManagedPaths(t *testing.T) { } } +func TestDryRunIncludesCanonicalTemplatePathsOutsideMemoryBank(t *testing.T) { + source, root := t.TempDir(), t.TempDir() + if err := os.MkdirAll(filepath.Join(source, "template", "memory-bank", "dna"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(source, "template", ".config"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(source, "template", "memory-bank", "dna", "rule.md"), []byte("base\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(source, "template", ".config", "hidden"), []byte("base\n"), 0o644); err != nil { + t.Fatal(err) + } + git(t, source, "init", "--quiet") + git(t, source, "add", ".") + git(t, source, "-c", "user.name=Test", "-c", "user.email=test@example.invalid", "commit", "--quiet", "-m", "template") + ref := git(t, source, "rev-parse", "HEAD") + if _, err := ownership.Init(ownership.Options{RepoRoot: root, SourceRoot: source, TemplateVersion: "v1", SourceRef: ref}); err != nil { + t.Fatal(err) + } + git(t, root, "init", "--quiet") + git(t, root, "add", ".") + git(t, root, "-c", "user.name=Test", "-c", "user.email=test@example.invalid", "commit", "--quiet", "-m", "downstream") + upstream := filepath.Join(root, "memory-bank", ".repo") + if err := os.MkdirAll(filepath.Join(upstream, "template", "memory-bank"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(upstream, "template", "memory-bank", ".keep"), []byte("base\n"), 0o644); err != nil { + t.Fatal(err) + } + git(t, upstream, "init", "--quiet") + git(t, upstream, "remote", "add", "origin", "https://github.com/example/upstream.git") + git(t, upstream, "add", ".") + git(t, upstream, "-c", "user.name=Test", "-c", "user.email=test@example.invalid", "commit", "--quiet", "-m", "base") + git(t, upstream, "update-ref", "refs/remotes/origin/master", "HEAD") + git(t, upstream, "symbolic-ref", "refs/remotes/origin/HEAD", "refs/remotes/origin/master") + if err := os.WriteFile(filepath.Join(root, ".config", "hidden"), []byte("changed\n"), 0o644); err != nil { + t.Fatal(err) + } + + report, err := Run(Options{RepoRoot: root, DryRun: true}) + if err != nil { + t.Fatal(err) + } + for _, decision := range report.Decisions { + if decision.Path == ".config/hidden" && decision.Action == "include" { + return + } + } + t.Fatalf("canonical path outside memory-bank was not included: %#v", report.Decisions) +} + func TestRejectsDirtyUpstreamCheckout(t *testing.T) { root := t.TempDir() upstream := filepath.Join(root, "memory-bank", ".repo") From a59fd78d1b3611d42a7c910f54af61719aa5c9b9 Mon Sep 17 00:00:00 2001 From: Danil Pismenny Date: Fri, 24 Jul 2026 19:22:37 +0300 Subject: [PATCH 3/5] fix: prefer canonical template for push --- internal/push/push.go | 6 ++++++ internal/push/push_test.go | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/internal/push/push.go b/internal/push/push.go index 67dc0e3..249575f 100644 --- a/internal/push/push.go +++ b/internal/push/push.go @@ -323,6 +323,9 @@ func selectPayloadRoot(checkout string) (string, error) { if !info.IsDir() || info.Mode()&os.ModeSymlink != 0 { return "", fmt.Errorf("upstream payload root %q must be a real directory", candidate) } + if candidate == "template" { + return candidate, nil + } roots = append(roots, candidate) } if len(roots) != 1 { @@ -339,6 +342,9 @@ func selectPayloadRootAt(run func(string, string, ...string) (string, error), ch return "", fmt.Errorf("inspect upstream payload root %q: %w", candidate, err) } if strings.TrimSpace(out) == candidate { + if candidate == "template" { + return candidate, nil + } roots = append(roots, candidate) } } diff --git a/internal/push/push_test.go b/internal/push/push_test.go index 05579cd..6f513f1 100644 --- a/internal/push/push_test.go +++ b/internal/push/push_test.go @@ -278,6 +278,27 @@ func TestDryRunIncludesCanonicalTemplatePathsOutsideMemoryBank(t *testing.T) { t.Fatalf("canonical path outside memory-bank was not included: %#v", report.Decisions) } +func TestCanonicalTemplateRootTakesPrecedenceOverLegacyRoots(t *testing.T) { + checkout := t.TempDir() + for _, directory := range []string{"template", "memory-bank-template", "memory-bank"} { + if err := os.Mkdir(filepath.Join(checkout, directory), 0o755); err != nil { + t.Fatal(err) + } + } + if got, err := selectPayloadRoot(checkout); err != nil || got != "template" { + t.Fatalf("worktree payload root = %q, %v; want template", got, err) + } + run := func(_ string, name string, args ...string) (string, error) { + if name != "git" || len(args) != 6 || args[0] != "ls-tree" { + return "", errors.New("unexpected command") + } + return args[len(args)-1], nil + } + if got, err := selectPayloadRootAt(run, checkout, "origin/main"); err != nil || got != "template" { + t.Fatalf("Git payload root = %q, %v; want template", got, err) + } +} + func TestRejectsDirtyUpstreamCheckout(t *testing.T) { root := t.TempDir() upstream := filepath.Join(root, "memory-bank", ".repo") From a9eac5f6d095b49f505dcac2ddc12686c3a945e1 Mon Sep 17 00:00:00 2001 From: Danil Pismenny Date: Fri, 24 Jul 2026 20:56:10 +0300 Subject: [PATCH 4/5] fix: reject absolute lock paths --- internal/ownership/lock.go | 2 +- internal/ownership/update_test.go | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/internal/ownership/lock.go b/internal/ownership/lock.go index 4ff81ff..f2600df 100644 --- a/internal/ownership/lock.go +++ b/internal/ownership/lock.go @@ -65,7 +65,7 @@ func readLockSnapshot(repo pinnedRepo) (Lock, bool, string, error) { return Lock{}, false, "", fmt.Errorf("invalid last update in %s", LockFileName) } for filePath, file := range lock.Files { - if filePath == LockFileName || strings.Contains(filePath, "\\") || path.Clean(filePath) != filePath || strings.HasPrefix(filePath, "../") || filePath == "." { + if filePath == LockFileName || path.IsAbs(filePath) || strings.Contains(filePath, "\\") || path.Clean(filePath) != filePath || strings.HasPrefix(filePath, "../") || filePath == "." { return Lock{}, false, "", fmt.Errorf("invalid path %q in %s", filePath, LockFileName) } switch file.Ownership { diff --git a/internal/ownership/update_test.go b/internal/ownership/update_test.go index e9799e0..9f8d22b 100644 --- a/internal/ownership/update_test.go +++ b/internal/ownership/update_test.go @@ -70,6 +70,21 @@ func TestAgentFileRejectsCaseAliasesOfMemoryBank(t *testing.T) { } } +func TestReadLockRejectsAbsoluteManagedPath(t *testing.T) { + repo, source := t.TempDir(), t.TempDir() + write(t, source, "memory-bank/dna/rule.md", "managed\n") + initialize(t, repo, source) + lockPath := filepath.Join(repo, filepath.FromSlash(LockFileName)) + lock := read(t, repo, LockFileName) + lock = strings.Replace(lock, "memory-bank/dna/rule.md", "/outside.md", 1) + if err := os.WriteFile(lockPath, []byte(lock), 0o644); err != nil { + t.Fatal(err) + } + if _, _, err := ReadLock(repo); err == nil || !strings.Contains(err.Error(), "invalid path") { + t.Fatalf("absolute lock path was accepted: %v", err) + } +} + func TestAgentPlanPreservesExplicitZeroMode(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("Windows does not expose Unix permission bits") From 271d30aa0c17a8c28bd357f2f2b0b74ba6ee9b91 Mon Sep 17 00:00:00 2001 From: Danil Pismenny Date: Fri, 24 Jul 2026 20:58:18 +0300 Subject: [PATCH 5/5] fix: preserve template-owned agent files --- internal/ownership/source_test.go | 34 +++++++++++++++++++++++++++++++ internal/ownership/update.go | 18 ++++++++++------ 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/internal/ownership/source_test.go b/internal/ownership/source_test.go index 54fa355..54b1921 100644 --- a/internal/ownership/source_test.go +++ b/internal/ownership/source_test.go @@ -238,6 +238,40 @@ func TestCanonicalTemplateIncludesAllTrackedFiles(t *testing.T) { } } +func TestCanonicalTemplateOwnsAgentFileWhenPresent(t *testing.T) { + source, repo := t.TempDir(), t.TempDir() + write(t, source, "template/AGENTS.md", "template instructions\n") + write(t, source, "template/memory-bank/dna/rule.md", "rule\n") + firstCommit := commitTestSource(t, source) + + report, err := Init(Options{RepoRoot: repo, SourceRoot: source, TemplateVersion: "v1", SourceRef: firstCommit}) + if err != nil || !report.Applied { + t.Fatalf("init failed: report=%#v err=%v", report, err) + } + if got := read(t, repo, "AGENTS.md"); got != "template instructions\n" { + t.Fatalf("template agent file = %q", got) + } + if strings.Contains(read(t, repo, "AGENTS.md"), "MEMORY BANK START") { + t.Fatal("template agent file was rewritten by the generated instruction plan") + } + lock, exists, err := ReadLock(repo) + if err != nil || !exists || lock.Files["AGENTS.md"].Ownership != Managed { + t.Fatalf("template agent file was not locked as managed: %#v, exists=%v, err=%v", lock, exists, err) + } + + write(t, source, "template/AGENTS.md", "updated template instructions\n") + runGitTest(t, source, "add", "--all") + runGitTest(t, source, "-c", "user.name=Memory Bank Tests", "-c", "user.email=tests@example.invalid", "commit", "--quiet", "-m", "update template instructions") + secondCommit := runGitTest(t, source, "rev-parse", "HEAD") + report, err = Update(Options{RepoRoot: repo, SourceRoot: source, TemplateVersion: "v2", SourceRef: secondCommit}) + if err != nil || !report.Applied || decisionFor(t, report, "AGENTS.md").Action != UpdateFile { + t.Fatalf("update failed: report=%#v err=%v", report, err) + } + if got := read(t, repo, "AGENTS.md"); got != "updated template instructions\n" { + t.Fatalf("updated template agent file = %q", got) + } +} + func TestPinnedSourceObjectsIgnoreHiddenWorktreeChanges(t *testing.T) { for _, test := range []struct { name string diff --git a/internal/ownership/update.go b/internal/ownership/update.go index 4e9b6d0..a6dd34b 100644 --- a/internal/ownership/update.go +++ b/internal/ownership/update.go @@ -117,13 +117,19 @@ func run(options Options, old Lock, hasLock bool, repo pinnedRepo, lockDigest st return Report{}, err } templateMutationCount := len(mutations) - agentMutation, agentDecision, err := buildAgentPlan(repo, options.AgentFile) - if err != nil { - return Report{}, err + agentTarget := options.AgentFile + if agentTarget == "" { + agentTarget = agentinstructions.DefaultTarget } - decisions = append(decisions, agentDecision) - if agentMutation != nil { - mutations = append(mutations, *agentMutation) + if _, templateOwnsAgentFile := source[filepath.ToSlash(agentTarget)]; !templateOwnsAgentFile { + agentMutation, agentDecision, err := buildAgentPlan(repo, options.AgentFile) + if err != nil { + return Report{}, err + } + decisions = append(decisions, agentDecision) + if agentMutation != nil { + mutations = append(mutations, *agentMutation) + } } report := Report{FormatVersion: ReportFormatVersion, DryRun: options.DryRun, Decisions: decisions} for _, decision := range decisions {