From 5fd28245a3d2938c2d9f4f8f13684143564b45a1 Mon Sep 17 00:00:00 2001 From: Danil Pismenny Date: Fri, 24 Jul 2026 19:10:37 +0300 Subject: [PATCH 1/6] 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 | 33 +++++++++++++++------ internal/push/push_test.go | 16 +++++++---- 9 files changed, 141 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 6c10d37..a9b8e08 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 59c0e7d..24b82cc 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -201,7 +201,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 d614a13..243d3f1 100644 --- a/internal/push/push.go +++ b/internal/push/push.go @@ -191,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)) } } @@ -308,7 +309,7 @@ func defaultBranch(run func(string, string, ...string) (string, error), checkout func selectPayloadRoot(checkout string) (string, error) { 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 @@ -322,14 +323,14 @@ func selectPayloadRoot(checkout string) (string, error) { roots = append(roots, candidate) } if len(roots) != 1 { - return "", fmt.Errorf("upstream checkout must contain exactly one payload root (memory-bank-template or memory-bank), found %v", 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) { 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) @@ -339,11 +340,27 @@ func selectPayloadRootAt(run func(string, string, ...string) (string, error), ch } } if len(roots) != 1 { - return "", fmt.Errorf("default branch must contain exactly one payload root (memory-bank-template or memory-bank), found %v", 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 { diff --git a/internal/push/push_test.go b/internal/push/push_test.go index 75a7d35..7e36cd0 100644 --- a/internal/push/push_test.go +++ b/internal/push/push_test.go @@ -31,7 +31,7 @@ func TestRunCreatesBranchCopiesManagedFileAndReturnsPR(t *testing.T) { return dir, nil } switch call { - case "git rev-parse --is-inside-work-tree", "git status --porcelain", "git diff --name-only --diff-filter=U", "git rev-parse --verify origin/main", "git add -- memory-bank-template/dna/rule.md", "git commit -m Publish managed Memory Bank changes", "git push -u origin memory-bank-cli/push-20260724-120000": + case "git rev-parse --is-inside-work-tree", "git status --porcelain", "git diff --name-only --diff-filter=U", "git rev-parse --verify origin/main", "git add -- template/memory-bank/dna/rule.md", "git commit -m Publish managed Memory Bank changes", "git push -u origin memory-bank-cli/push-20260724-120000": return "", nil case "git remote get-url origin": return "https://github.com/example/upstream.git", nil @@ -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": + return "template", nil case "git ls-tree -d --name-only origin/main -- memory-bank-template": - return "memory-bank-template", nil + 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": @@ -71,7 +73,7 @@ func TestRunCreatesBranchCopiesManagedFileAndReturnsPR(t *testing.T) { if report.Branch != "memory-bank-cli/push-20260724-120000" || report.PRURL != "https://github.com/example/upstream/pull/1" { t.Fatalf("unexpected report: %#v", report) } - data, err := os.ReadFile(filepath.Join(checkout, "memory-bank-template", "dna", "rule.md")) + data, err := os.ReadFile(filepath.Join(checkout, "template", "memory-bank", "dna", "rule.md")) if err != nil || string(data) != "changed\n" { t.Fatalf("managed file was not copied: %q, %v", data, err) } @@ -95,7 +97,7 @@ func TestRunCompensatesRemoteBranchWhenPRCreationFails(t *testing.T) { return dir, nil } switch call { - case "git rev-parse --is-inside-work-tree", "git status --porcelain", "git diff --name-only --diff-filter=U", "git rev-parse --verify origin/main", "git add -- memory-bank-template/dna/rule.md", "git commit -m Publish managed Memory Bank changes", "git push -u origin memory-bank-cli/push-20260724-120000", "git push origin --delete memory-bank-cli/push-20260724-120000", "git reset --hard", "git checkout main", "git branch -D memory-bank-cli/push-20260724-120000", "git reset --hard abc123", "git clean -fd -- memory-bank-template/dna/rule.md": + case "git rev-parse --is-inside-work-tree", "git status --porcelain", "git diff --name-only --diff-filter=U", "git rev-parse --verify origin/main", "git add -- template/memory-bank/dna/rule.md", "git commit -m Publish managed Memory Bank changes", "git push -u origin memory-bank-cli/push-20260724-120000", "git push origin --delete memory-bank-cli/push-20260724-120000", "git reset --hard", "git checkout main", "git branch -D memory-bank-cli/push-20260724-120000", "git reset --hard abc123", "git clean -fd -- template/memory-bank/dna/rule.md": return "", nil case "git remote get-url origin": return "https://github.com/example/upstream.git", nil @@ -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": + return "template", nil case "git ls-tree -d --name-only origin/main -- memory-bank-template": - return "memory-bank-template", nil + 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": @@ -146,7 +150,7 @@ func pushFixture(t *testing.T) string { if err := os.MkdirAll(filepath.Join(root, "memory-bank", ".repo"), 0o755); err != nil { t.Fatal(err) } - if err := os.MkdirAll(filepath.Join(root, "memory-bank", ".repo", "memory-bank-template"), 0o755); err != nil { + if err := os.MkdirAll(filepath.Join(root, "memory-bank", ".repo", "template", "memory-bank"), 0o755); err != nil { t.Fatal(err) } if err := os.WriteFile(filepath.Join(root, "memory-bank", "dna", "rule.md"), []byte("changed\n"), 0o644); err != nil { From 641be4016247df79d16171e779388fcb84ad9268 Mon Sep 17 00:00:00 2001 From: Danil Pismenny Date: Fri, 24 Jul 2026 19:16:39 +0300 Subject: [PATCH 2/6] 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 dd065e7e754ac48e736d9d8534f982c49cf636f2 Mon Sep 17 00:00:00 2001 From: Danil Pismenny Date: Fri, 24 Jul 2026 19:22:37 +0300 Subject: [PATCH 3/6] 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 e1373312a83c8c8ad0985af740ccf273732bc9ac Mon Sep 17 00:00:00 2001 From: Danil Pismenny Date: Fri, 24 Jul 2026 20:56:10 +0300 Subject: [PATCH 4/6] 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 4d50f716ff6c0bd8a90a10b0406a807257865980 Mon Sep 17 00:00:00 2001 From: Danil Pismenny Date: Fri, 24 Jul 2026 20:58:18 +0300 Subject: [PATCH 5/6] 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 { From 0dbad719b260427148bbfe7748718371a78894e0 Mon Sep 17 00:00:00 2001 From: Danil Pismenny Date: Fri, 24 Jul 2026 21:15:30 +0300 Subject: [PATCH 6/6] fix: complete full template payload model --- README.md | 9 ++- internal/cli/cli.go | 2 +- internal/doctor/doctor.go | 12 +++- internal/doctor/doctor_test.go | 57 +++++++++++++++ internal/ownership/lock.go | 3 +- internal/ownership/payload_path.go | 28 ++++++++ internal/ownership/payload_path_test.go | 21 ++++++ internal/ownership/source.go | 8 +-- internal/ownership/source_test.go | 15 ++++ internal/ownership/update.go | 25 ++++--- internal/ownership/update_test.go | 55 +++++++++++++++ internal/push/push.go | 19 ++--- internal/push/push_test.go | 69 ++++++++++++++++++- memory-bank/engineering/architecture.md | 30 ++++++-- .../design.md | 13 ++-- 15 files changed, 322 insertions(+), 44 deletions(-) create mode 100644 internal/ownership/payload_path.go create mode 100644 internal/ownership/payload_path_test.go diff --git a/README.md b/README.md index a9b8e08..13c3f20 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,9 @@ CLI for installing, updating, validating, and diagnosing Memory Bank templates `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. +Existing locks from the legacy payload roots are migrated conservatively: +unchanged files adopt canonical ownership, while local customization is +preserved for explicit resolution. ## Publish managed changes upstream @@ -14,7 +17,11 @@ From a downstream Git repository with a clean upstream checkout at `memory-bank/ memory-bank-cli push --dry-run ``` -Without `--dry-run`, `push` creates a fresh upstream branch, commits only changed `managed` Memory Bank paths, pushes it and creates a GitHub PR. It never pushes the upstream default branch directly. Non-managed paths, including project artifacts, lock/state and `.repo`, are reported as exclusions. +Without `--dry-run`, `push` creates a fresh upstream branch, publishes every +changed path recorded as `managed` in the ownership lock back below +`template/`, pushes the branch and creates a GitHub PR. It never pushes the +upstream default branch directly. Non-managed paths, including project +artifacts, lock/state and `.repo`, are reported as exclusions. ## Install diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 24b82cc..ce82531 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -89,7 +89,7 @@ func printRootUsage(writer io.Writer) { fmt.Fprintln(writer, " doctor Diagnose adoption, governance, managed drift, and navigation") fmt.Fprintln(writer, " lint Audit markdown navigation integrity") fmt.Fprintln(writer, " github Install or update the optional GitHub workflow adapter") - fmt.Fprintln(writer, " push Publish managed Memory Bank changes upstream through a PR") + fmt.Fprintln(writer, " push Publish locked canonical template changes upstream through a PR") fmt.Fprintln(writer) fmt.Fprintln(writer, "Options:") fmt.Fprintln(writer, " --help Show this help") diff --git a/internal/doctor/doctor.go b/internal/doctor/doctor.go index fa3bd85..4f24f68 100644 --- a/internal/doctor/doctor.go +++ b/internal/doctor/doctor.go @@ -28,7 +28,7 @@ var shellErrexitPattern = regexp.MustCompile(`(?:^|\s)-[A-Za-z]*e[A-Za-z]*(?:\s| var shellPipefailPattern = regexp.MustCompile(`(?:^|\s)-o\s+pipefail(?:\s|$)|(?:^|\s)pipefail(?:\s|$)`) var workflowFalseExpressionPattern = regexp.MustCompile(`^\$\{\{\s*false\s*\}\}$`) -const templatePayloadRoot = "template/memory-bank" +const templateScopeRoot = "template/memory-bank" func NormalizeProfile(value string) (Profile, error) { profile := Profile(strings.ToLower(strings.TrimSpace(value))) @@ -53,7 +53,7 @@ func Run(options Options) (Report, error) { scopeRoot := options.ScopeRoot if scopeRoot == "" { if profile == ProfileTemplate { - scopeRoot = templatePayloadRoot + scopeRoot = templateScopeRoot } else { scopeRoot = "memory-bank" } @@ -101,7 +101,7 @@ func detectProfile(repoRoot string) Profile { if _, err := os.Lstat(filepath.Join(repoRoot, ownership.LockFileName)); err == nil { return ProfileDownstream } - info, err := os.Lstat(filepath.Join(repoRoot, templatePayloadRoot)) + info, err := os.Lstat(filepath.Join(repoRoot, ownership.CanonicalTemplateRoot)) if err != nil || info.Mode()&os.ModeSymlink != 0 || !info.IsDir() { return ProfileDownstream } @@ -112,18 +112,24 @@ func (report *Report) add(finding Finding) { report.Findings = append(report.Fin func (report *Report) checkIdentityAndDrift(agentFile, scopeRoot string) { lock, exists, lockErr := ownership.ReadLock(report.RepoRoot) + templateOwnsAgentFile := false if lockErr != nil { report.add(Finding{Code: "manifest.invalid", Severity: Error, Group: "manifest", Path: ownership.LockFileName, Message: lockErr.Error(), Remediation: "Repair or recreate the ownership lock with memory-bank-cli init from a trusted template checkout."}) } else if exists { report.TemplateIdentity = TemplateIdentity{SchemaVersion: lock.SchemaVersion, Version: lock.Template.Version, SourceRef: lock.Template.SourceRef} report.add(Finding{Code: "template.identity", Severity: Info, Group: "template_identity", Path: ownership.LockFileName, Subject: lock.Template.Version, Message: "Installed template identity is recorded in the ownership lock.", Remediation: "Use memory-bank-cli update --dry-run to compare with a newer pinned template source."}) report.checkManagedDrift(lock) + agentContract, tracked := lock.Files[filepath.ToSlash(agentFile)] + templateOwnsAgentFile = tracked && agentContract.Ownership == ownership.Managed } else if report.Profile == ProfileDownstream { report.add(Finding{Code: "template.identity_missing", Severity: Error, Group: "template_identity", Path: ownership.LockFileName, Message: "The downstream repository has no ownership lock, so its installed template version is unknown.", Remediation: "Adopt the template with memory-bank-cli init and commit memory-bank/.lock."}) } else { report.add(Finding{Code: "template.source_repository", Severity: Info, Group: "template_identity", Subject: "template", Message: "Template source profile detected; an installed-template lock is not expected.", Remediation: "Create locks only in downstream repositories through memory-bank-cli init."}) } + if templateOwnsAgentFile { + return + } contents, _, err := readRegularWithinRoot(report.RepoRoot, agentFile) if err != nil { severity := Error diff --git a/internal/doctor/doctor_test.go b/internal/doctor/doctor_test.go index 295ca80..79f1c17 100644 --- a/internal/doctor/doctor_test.go +++ b/internal/doctor/doctor_test.go @@ -135,6 +135,16 @@ func TestImplicitScopeFollowsResolvedProfile(t *testing.T) { } } +func TestDetectProfileUsesWholeCanonicalTemplateRoot(t *testing.T) { + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, ownership.CanonicalTemplateRoot, ".config"), 0o755); err != nil { + t.Fatal(err) + } + if got := detectProfile(root); got != ProfileTemplate { + t.Fatalf("profile with canonical template root = %q, want %q", got, ProfileTemplate) + } +} + func TestProfileDetectionUsesTemplatePayloadRoot(t *testing.T) { for _, test := range []struct { name string @@ -275,6 +285,53 @@ func TestManagedDriftProducesStableFinding(t *testing.T) { t.Fatalf("managed drift finding missing: %#v", report.Findings) } +func TestTemplateOwnedAgentFileUsesLockDriftContract(t *testing.T) { + source, repo := t.TempDir(), t.TempDir() + writeSource := func(relative, contents string) { + t.Helper() + target := filepath.Join(source, filepath.FromSlash(relative)) + if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(target, []byte(contents), 0o644); err != nil { + t.Fatal(err) + } + } + writeSource("template/AGENTS.md", "canonical template instructions\n") + writeSource("template/memory-bank/README.md", "---\ndoc_function: index\npurpose: Test Memory Bank.\nstatus: active\n---\n# Memory Bank\n") + runGit(t, source, "init", "--quiet") + runGit(t, source, "add", "--all") + runGit(t, source, "-c", "user.name=Doctor Test", "-c", "user.email=doctor@example.invalid", "commit", "--quiet", "-m", "template") + ref := strings.TrimSpace(runGit(t, source, "rev-parse", "HEAD")) + if _, err := ownership.Init(ownership.Options{RepoRoot: repo, SourceRoot: source, TemplateVersion: "v1", SourceRef: ref}); err != nil { + t.Fatal(err) + } + + report, err := Run(Options{RepoRoot: repo, ScopeRoot: "memory-bank", AgentFile: "AGENTS.md", Profile: ProfileAuto, MaxDepth: 3}) + if err != nil { + t.Fatal(err) + } + for _, finding := range report.Findings { + if strings.HasPrefix(finding.Code, "agent.") { + t.Fatalf("template-owned agent file received generated-block finding: %#v", finding) + } + } + + if err := os.WriteFile(filepath.Join(repo, "AGENTS.md"), []byte("local drift\n"), 0o644); err != nil { + t.Fatal(err) + } + report, err = Run(Options{RepoRoot: repo, ScopeRoot: "memory-bank", AgentFile: "AGENTS.md", Profile: ProfileAuto, MaxDepth: 3}) + if err != nil { + t.Fatal(err) + } + for _, finding := range report.Findings { + if finding.Code == "manifest.managed_content_drift" && finding.Path == "AGENTS.md" { + return + } + } + t.Fatalf("template-owned agent drift was not reported through the lock: %#v", report.Findings) +} + func TestGovernanceCycleAndLifecycleFindingsHaveStableCodes(t *testing.T) { repo := t.TempDir() write := func(relative, contents string) { diff --git a/internal/ownership/lock.go b/internal/ownership/lock.go index f2600df..363e848 100644 --- a/internal/ownership/lock.go +++ b/internal/ownership/lock.go @@ -65,7 +65,8 @@ 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 || path.IsAbs(filePath) || strings.Contains(filePath, "\\") || path.Clean(filePath) != filePath || strings.HasPrefix(filePath, "../") || filePath == "." { + firstComponent := strings.SplitN(filePath, "/", 2)[0] + if filePath == LockFileName || path.IsAbs(filePath) || strings.Contains(filePath, "\\") || path.Clean(filePath) != filePath || strings.HasPrefix(filePath, "../") || filePath == "." || strings.EqualFold(firstComponent, ".git") { return Lock{}, false, "", fmt.Errorf("invalid path %q in %s", filePath, LockFileName) } switch file.Ownership { diff --git a/internal/ownership/payload_path.go b/internal/ownership/payload_path.go new file mode 100644 index 0000000..526474e --- /dev/null +++ b/internal/ownership/payload_path.go @@ -0,0 +1,28 @@ +package ownership + +import ( + "path" + "path/filepath" + "strings" +) + +const ( + // CanonicalTemplateRoot is the complete tracked source payload tree. + CanonicalTemplateRoot = "template" + // DownstreamPayloadRoot is retained for legacy source-root translation and + // for the ownership lock stored below memory-bank/. + DownstreamPayloadRoot = "memory-bank" +) + +// CanonicalDownstreamPath maps a path relative to template/ into a downstream +// repository. The mapping is deliberately name-agnostic: stripping the +// canonical root preserves every relative component, including memory-bank/. +func CanonicalDownstreamPath(sourceRelative string) string { + return strings.TrimPrefix(filepath.ToSlash(sourceRelative), "/") +} + +// CanonicalTemplatePath is the inverse mapping used when publishing a locked +// downstream path into the canonical template tree. +func CanonicalTemplatePath(downstreamPath string) string { + return path.Join(CanonicalTemplateRoot, filepath.ToSlash(downstreamPath)) +} diff --git a/internal/ownership/payload_path_test.go b/internal/ownership/payload_path_test.go new file mode 100644 index 0000000..4f29989 --- /dev/null +++ b/internal/ownership/payload_path_test.go @@ -0,0 +1,21 @@ +package ownership + +import "testing" + +func TestCanonicalPayloadPathRoundTripsWithoutPathRules(t *testing.T) { + for _, sourceRelative := range []string{ + "memory-bank/dna/rule.md", + ".config/hidden", + ".github/workflows/check.yml", + "AGENTS.md", + "nested/new/path/tool", + } { + downstream := CanonicalDownstreamPath(sourceRelative) + if downstream != sourceRelative { + t.Fatalf("downstream path for %q = %q", sourceRelative, downstream) + } + if got, want := CanonicalTemplatePath(downstream), CanonicalTemplateRoot+"/"+sourceRelative; got != want { + t.Fatalf("canonical round trip for %q = %q, want %q", sourceRelative, got, want) + } + } +} diff --git a/internal/ownership/source.go b/internal/ownership/source.go index 7674e11..bcada83 100644 --- a/internal/ownership/source.go +++ b/internal/ownership/source.go @@ -18,8 +18,8 @@ type pinnedSource struct { const ( legacySourcePayloadRoot = "memory-bank" legacyTemplateSourcePayloadRoot = "memory-bank-template" - targetSourcePayloadRoot = "template" - downstreamPayloadRoot = "memory-bank" + targetSourcePayloadRoot = CanonicalTemplateRoot + downstreamPayloadRoot = DownstreamPayloadRoot ) func pinSourceRoot(root string) (pinnedSource, error) { @@ -109,7 +109,7 @@ func verifySourceCheckout(root, expectedRef string) error { return fmt.Errorf("inspect source template status: %w", err) } if payloadStatus != "" { - return errors.New("source memory-bank tree contains uncommitted or ignored payloads") + return errors.New("source template tree contains uncommitted or ignored payloads") } if err := verifySourcePayload(root, expectedRef, payloadRoot); err != nil { return err @@ -134,7 +134,7 @@ func verifySourcePayload(root, expectedRef, payloadRoot string) error { } mode, objectType, objectID := fields[0], fields[1], fields[2] if objectType != "blob" || mode != "100644" && mode != "100755" { - return fmt.Errorf("pinned source payload contains unsupported entry: %q", path) + return fmt.Errorf("pinned source payload contains unsupported Git entry %q (mode %s, type %s); only regular files with modes 100644 or 100755 are supported", path, mode, objectType) } expected[path] = objectID } diff --git a/internal/ownership/source_test.go b/internal/ownership/source_test.go index 54b1921..4f53b63 100644 --- a/internal/ownership/source_test.go +++ b/internal/ownership/source_test.go @@ -238,6 +238,21 @@ func TestCanonicalTemplateIncludesAllTrackedFiles(t *testing.T) { } } +func TestCanonicalTemplateRejectsTrackedSymlink(t *testing.T) { + source, repo := t.TempDir(), t.TempDir() + write(t, source, "template/memory-bank/dna/rule.md", "rule\n") + symlinkForTest(t, "memory-bank/dna/rule.md", filepath.Join(source, "template", "alias")) + commit := commitTestSource(t, source) + + report, err := Init(Options{RepoRoot: repo, SourceRoot: source, TemplateVersion: "v1", SourceRef: commit}) + if err == nil || !strings.Contains(err.Error(), "unsupported Git entry") { + t.Fatalf("tracked symlink was accepted: report=%#v err=%v", report, err) + } + if _, statErr := os.Lstat(filepath.Join(repo, LockFileName)); !os.IsNotExist(statErr) { + t.Fatalf("failed init created a lock: %v", statErr) + } +} + func TestCanonicalTemplateOwnsAgentFileWhenPresent(t *testing.T) { source, repo := t.TempDir(), t.TempDir() write(t, source, "template/AGENTS.md", "template instructions\n") diff --git a/internal/ownership/update.go b/internal/ownership/update.go index a6dd34b..a57fea4 100644 --- a/internal/ownership/update.go +++ b/internal/ownership/update.go @@ -340,7 +340,7 @@ func readGitSource(source pinnedSource, ref string) (map[string]payload, error) header, filePath, found := strings.Cut(record, "\t") fields := strings.Fields(header) if !found || len(fields) != 3 || fields[1] != "blob" || fields[0] != "100644" && fields[0] != "100755" { - return nil, fmt.Errorf("read pinned source tree: unsupported entry %q", filePath) + return nil, fmt.Errorf("read pinned source tree: unsupported Git entry %q; only regular files with modes 100644 or 100755 are supported", filePath) } data, err := gitBytes(source.root, "cat-file", "blob", fields[2]) if err != nil { @@ -366,13 +366,7 @@ func downstreamPayloadPath(payloadRoot, relative string) string { if payloadRoot != targetSourcePayloadRoot { return downstreamPayloadRoot + "/" + relative } - if relative == downstreamPayloadRoot { - return downstreamPayloadRoot - } - if strings.HasPrefix(relative, downstreamPayloadRoot+"/") { - return relative - } - return relative + return CanonicalDownstreamPath(relative) } func sourcePayloadClass(payloadRoot, relative string) Class { @@ -543,6 +537,21 @@ func buildPlan(repo pinnedRepo, source map[string]payload, old Lock, hasLock boo decision.Action, decision.Reason = Preserve, "untracked existing file is downstream-owned" file = File{Ownership: UserOwned} } + case class == Managed && (prior.Ownership == Adapted || prior.Ownership == UserOwned): + downstreamChanged := prior.BaseDigest == "" || currentDigest != prior.BaseDigest || !modeMatches(currentMode, priorBaseMode) + upstreamChanged := prior.BaseDigest == "" || incoming.digest != prior.BaseDigest || incoming.mode != priorBaseMode + switch { + case currentDigest == incoming.digest && modeMatches(currentMode, incoming.mode): + decision.Action, decision.Reason = Preserve, "adopt matching legacy-owned file as canonical managed payload" + case downstreamChanged && upstreamChanged: + decision.Action, decision.Reason = Conflict, "legacy-owned file changed both upstream and downstream during canonical migration" + file = prior + case upstreamChanged: + decision.Action, decision.Reason = UpdateFile, "unmodified legacy-owned file follows canonical managed payload" + default: + decision.Action, decision.Reason = Preserve, "preserve legacy downstream customization during canonical migration" + file = prior + } case class == UserOwned || prior.Ownership == UserOwned: decision.Action, decision.Reason = Preserve, "user-owned files are never overwritten" file = prior diff --git a/internal/ownership/update_test.go b/internal/ownership/update_test.go index 9f8d22b..3e48024 100644 --- a/internal/ownership/update_test.go +++ b/internal/ownership/update_test.go @@ -85,6 +85,21 @@ func TestReadLockRejectsAbsoluteManagedPath(t *testing.T) { } } +func TestReadLockRejectsGitMetadataPath(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", ".GiT/config", 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("Git metadata lock path was accepted: %v", err) + } +} + func TestAgentPlanPreservesExplicitZeroMode(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("Windows does not expose Unix permission bits") @@ -206,6 +221,46 @@ func TestLegacyLockMigratesToCanonicalTemplateWithoutRemovingManagedFiles(t *tes } } +func TestLegacyLockMigrationPromotesOnlySafeCanonicalFiles(t *testing.T) { + repo, source := t.TempDir(), t.TempDir() + adaptedClean := "memory-bank/domain/model.md" + userClean := "memory-bank/features/FT-001/brief.md" + userDrift := "memory-bank/features/FT-001/local.md" + for _, path := range []string{adaptedClean, userClean, userDrift} { + write(t, source, path, "v1\n") + } + initialize(t, repo, source) + write(t, repo, userDrift, "local customization\n") + + if err := os.RemoveAll(filepath.Join(source, "memory-bank")); err != nil { + t.Fatal(err) + } + write(t, source, "template/"+adaptedClean, "v2\n") + write(t, source, "template/"+userClean, "v1\n") + write(t, source, "template/"+userDrift, "v1\n") + + report, err := Update(opts(repo, source, "b")) + if err != nil || !report.Applied || report.ConflictCount != 0 { + t.Fatalf("canonical migration failed: report=%#v err=%v", report, err) + } + if got := read(t, repo, adaptedClean); got != "v2\n" { + t.Fatalf("clean adapted file was not updated: %q", got) + } + if got := read(t, repo, userDrift); got != "local customization\n" { + t.Fatalf("legacy user customization was overwritten: %q", got) + } + lock, exists, err := ReadLock(repo) + if err != nil || !exists { + t.Fatalf("read migrated lock: exists=%v err=%v", exists, err) + } + if lock.Files[adaptedClean].Ownership != Managed || lock.Files[userClean].Ownership != Managed { + t.Fatalf("safe legacy files were not promoted: %#v", lock.Files) + } + if lock.Files[userDrift].Ownership != UserOwned { + t.Fatalf("drifted legacy file lost its ownership protection: %#v", lock.Files[userDrift]) + } +} + 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 249575f..83303e3 100644 --- a/internal/push/push.go +++ b/internal/push/push.go @@ -312,7 +312,7 @@ func defaultBranch(run func(string, string, ...string) (string, error), checkout func selectPayloadRoot(checkout string) (string, error) { var roots []string - for _, candidate := range []string{"template", "memory-bank-template", "memory-bank"} { + for _, candidate := range []string{ownership.CanonicalTemplateRoot, "memory-bank-template", ownership.DownstreamPayloadRoot} { info, err := os.Lstat(filepath.Join(checkout, candidate)) if errors.Is(err, os.ErrNotExist) { continue @@ -323,7 +323,7 @@ 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" { + if candidate == ownership.CanonicalTemplateRoot { return candidate, nil } roots = append(roots, candidate) @@ -336,13 +336,13 @@ func selectPayloadRoot(checkout string) (string, error) { func selectPayloadRootAt(run func(string, string, ...string) (string, error), checkout, ref string) (string, error) { var roots []string - for _, candidate := range []string{"template", "memory-bank-template", "memory-bank"} { + for _, candidate := range []string{ownership.CanonicalTemplateRoot, "memory-bank-template", ownership.DownstreamPayloadRoot} { 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) } if strings.TrimSpace(out) == candidate { - if candidate == "template" { + if candidate == ownership.CanonicalTemplateRoot { return candidate, nil } roots = append(roots, candidate) @@ -354,22 +354,17 @@ func selectPayloadRootAt(run func(string, string, ...string) (string, error), ch 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) - } return filepath.Join(checkout, payloadRoot) } func payloadDestinationPath(checkout, payloadRoot, downstreamPath string) (string, string) { - if payloadRoot != "template" { - relative := strings.TrimPrefix(downstreamPath, "memory-bank/") + if payloadRoot != ownership.CanonicalTemplateRoot { + relative := strings.TrimPrefix(downstreamPath, ownership.DownstreamPayloadRoot+"/") stagePath := filepath.ToSlash(filepath.Join(payloadRoot, relative)) return filepath.Join(checkout, filepath.FromSlash(stagePath)), stagePath } - stagePath := filepath.ToSlash(filepath.Join(payloadRoot, downstreamPath)) + stagePath := ownership.CanonicalTemplatePath(downstreamPath) return filepath.Join(checkout, filepath.FromSlash(stagePath)), stagePath } diff --git a/internal/push/push_test.go b/internal/push/push_test.go index 6f513f1..4b12d3f 100644 --- a/internal/push/push_test.go +++ b/internal/push/push_test.go @@ -1,10 +1,14 @@ package push import ( + "crypto/sha256" + "encoding/json" "errors" + "fmt" "os" "os/exec" "path/filepath" + "runtime" "strings" "testing" "time" @@ -24,6 +28,14 @@ func git(t *testing.T, dir string, args ...string) string { func TestRunCreatesBranchCopiesManagedFileAndReturnsPR(t *testing.T) { root := pushFixture(t) + tool := filepath.Join(root, ".config", "tool") + if err := os.MkdirAll(filepath.Dir(tool), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(tool, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + writeManagedLock(t, root, "memory-bank/dna/rule.md", ".config/tool") checkout := filepath.Join(root, "memory-bank", ".repo") var calls []string run := func(dir, name string, args ...string) (string, error) { @@ -33,7 +45,7 @@ func TestRunCreatesBranchCopiesManagedFileAndReturnsPR(t *testing.T) { return dir, nil } switch call { - case "git rev-parse --is-inside-work-tree", "git status --porcelain", "git diff --name-only --diff-filter=U", "git rev-parse --verify origin/main", "git add -- template/memory-bank/dna/rule.md", "git commit -m Publish managed Memory Bank changes", "git push -u origin memory-bank-cli/push-20260724-120000": + case "git rev-parse --is-inside-work-tree", "git status --porcelain", "git diff --name-only --diff-filter=U", "git rev-parse --verify origin/main", "git add -- template/.config/tool template/memory-bank/dna/rule.md", "git commit -m Publish managed Memory Bank changes", "git push -u origin memory-bank-cli/push-20260724-120000": return "", nil case "git remote get-url origin": return "https://github.com/example/upstream.git", nil @@ -56,7 +68,7 @@ func TestRunCreatesBranchCopiesManagedFileAndReturnsPR(t *testing.T) { case "git ls-tree -d --name-only origin/main -- memory-bank": return "", nil 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 + return " M .config/tool\x00 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 { t.Fatalf("branch created outside checkout: %q", dir) @@ -79,7 +91,20 @@ func TestRunCreatesBranchCopiesManagedFileAndReturnsPR(t *testing.T) { if err != nil || string(data) != "changed\n" { t.Fatalf("managed file was not copied: %q, %v", data, err) } - if _, err := os.Stat(filepath.Join(checkout, "memory-bank", "product", "note.md")); !os.IsNotExist(err) { + data, err = os.ReadFile(filepath.Join(checkout, "template", ".config", "tool")) + if err != nil || string(data) != "#!/bin/sh\n" { + t.Fatalf("canonical root file was not copied: %q, %v", data, err) + } + if runtime.GOOS != "windows" { + info, statErr := os.Stat(filepath.Join(checkout, "template", ".config", "tool")) + if statErr != nil { + t.Fatal(statErr) + } + if info.Mode().Perm() != 0o755 { + t.Fatalf("canonical executable mode = %v", info.Mode()) + } + } + if _, err := os.Stat(filepath.Join(checkout, "template", "memory-bank", "product", "note.md")); !os.IsNotExist(err) { t.Fatalf("excluded path was copied: %v", err) } for _, call := range calls { @@ -161,6 +186,44 @@ func pushFixture(t *testing.T) string { return root } +func writeManagedLock(t *testing.T, root string, paths ...string) { + t.Helper() + files := make(map[string]ownership.File, len(paths)) + for _, relative := range paths { + data, err := os.ReadFile(filepath.Join(root, filepath.FromSlash(relative))) + if err != nil { + t.Fatal(err) + } + value := fmt.Sprintf("sha256:%x", sha256.Sum256(data)) + mode := "100644" + info, err := os.Stat(filepath.Join(root, filepath.FromSlash(relative))) + if err != nil { + t.Fatal(err) + } + if info.Mode().Perm()&0o111 != 0 { + mode = "100755" + } + files[relative] = ownership.File{Ownership: ownership.Managed, BaseDigest: value, PayloadDigest: value, BaseMode: mode, PayloadMode: mode} + } + lock := ownership.Lock{ + SchemaVersion: ownership.CurrentSchemaVersion, + Template: ownership.Template{Version: "v1", SourceRef: strings.Repeat("a", 40)}, + LastUpdate: ownership.UpdateRecord{Version: "v1", At: time.Date(2026, 7, 24, 12, 0, 0, 0, time.UTC)}, + Files: files, + } + data, err := json.Marshal(lock) + if err != nil { + t.Fatal(err) + } + target := filepath.Join(root, filepath.FromSlash(ownership.LockFileName)) + if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(target, data, 0o644); err != nil { + t.Fatal(err) + } +} + func TestDryRunIncludesOnlyManagedPaths(t *testing.T) { root := t.TempDir() if err := os.MkdirAll(filepath.Join(root, "memory-bank", "dna"), 0o755); err != nil { diff --git a/memory-bank/engineering/architecture.md b/memory-bank/engineering/architecture.md index 0150d13..338d1e0 100644 --- a/memory-bank/engineering/architecture.md +++ b/memory-bank/engineering/architecture.md @@ -21,9 +21,10 @@ One executable, `cmd/memory-bank-cli`, imports `internal/cli` and exits with its ```text cmd/memory-bank-cli -> internal/cli -internal/cli -> internal/{ownership, doctor, lint, repository} +internal/cli -> internal/{ownership, doctor, lint, push, repository} internal/doctor -> ownership + lint/governance inspection internal/ownership -> Git source + local filesystem + memory-bank/.lock +internal/push -> ownership payload-path model + local/upstream Git + GitHub PR ``` ## Module boundaries @@ -34,6 +35,7 @@ internal/ownership -> Git source + local filesystem + memory-bank/.lock | `internal/ownership` | Template source validation, ownership classes, lock, plan and transactional apply. | | `internal/doctor` | Profile-driven read-only findings for adoption, governance, drift and navigation. | | `internal/lint` | Markdown parsing, navigation audit and reports. | +| `internal/push` | Lock-backed selection and compensating upstream branch/PR publication. | | `internal/repository` | Explicit/nearest-Git repository-root resolution. | | `internal/agentinstructions` | Managed block planning for one agent instruction file. | @@ -42,12 +44,30 @@ internal/ownership -> Git source + local filesystem + memory-bank/.lock - Safety: source/repository pinning, clean-ref verification, symlink-aware destination handling and atomic/rollback update paths. - Contract stability: CLI output supports text and versioned JSON reports; tests assert public fields and exit behavior. - Portability: Unix and Windows secure-path variants exist. -- Local operation: the CLI operates on filesystem and Git checkout inputs; no remote runtime is required by the visible implementation. +- Bounded external effects: init/update/doctor/lint remain local; `push` + explicitly crosses the configured upstream Git and GitHub boundaries through + a new branch and PR. -## Doctor template-profile detection +## Template payload and doctor profile detection -For `doctor --profile auto`, a repository is the template source when it has a real root directory `template/memory-bank/` and no `memory-bank/.lock`. The lock takes precedence and always selects downstream; a missing, non-directory or symlinked source root also selects downstream. Explicit `--profile template` and `--profile downstream` bypass this detector. +For `doctor --profile auto`, a repository is the template source when it has a +real root directory `template/` and no `memory-bank/.lock`. The lock takes +precedence and always selects downstream; a missing, non-directory or symlinked +source root also selects downstream. Explicit `--profile template` and +`--profile downstream` bypass this detector. The implicit documentation audit +scope for the template profile remains `template/memory-bank/`. -During the coordinated rename, source inspection accepts exactly one payload root—legacy `memory-bank/`, legacy `memory-bank-template/`, or target `template/memory-bank/`—and translates each accepted source-relative path to downstream `memory-bank/` before ownership planning. A source with none or multiple recognized roots is rejected; locks, installed navigation and generated agent guidance remain downstream `memory-bank/` contracts. +The canonical source payload is every tracked regular Git file below +`template/`. Its deterministic downstream path is the same relative suffix +after stripping `template/`, so `template/memory-bank/**` remains +`memory-bank/**` while dotfiles and arbitrary new nested paths require no code +change. The inverse mapping prefixes a lock-managed downstream path with +`template/` for `push`. Both directions share the ownership payload-path model. + +Legacy source roots `memory-bank-template/` and `memory-bank/` remain migration +inputs and map below downstream `memory-bank/`. Canonical `template/` takes +precedence when it coexists with a project-local legacy tree. Ownership locks +may cover safe repository-relative paths outside `memory-bank/`; Git metadata, +the lock itself, symlinks and non-regular Git entries are rejected. `memory-bank/` denotes payload data, not an executable name. diff --git a/memory-bank/features/FT-023-push-upstream-publication/design.md b/memory-bank/features/FT-023-push-upstream-publication/design.md index 730d021..b3aba81 100644 --- a/memory-bank/features/FT-023-push-upstream-publication/design.md +++ b/memory-bank/features/FT-023-push-upstream-publication/design.md @@ -6,6 +6,7 @@ purpose: "Feature-local solution for safe upstream publication: managed-only sel derived_from: - brief.md - decision-log.md + - "https://github.com/dapi/memory-bank-cli/issues/29" status: active audience: humans_and_agents must_not_define: @@ -36,7 +37,7 @@ must_not_define: ```mermaid flowchart LR operator["Operator"] --> cli["memory-bank-cli push"] - cli --> downstream["Current Git repository\nmemory-bank/"] + cli --> downstream["Current Git repository\nlock-managed paths"] cli --> checkout["Upstream checkout\nmemory-bank/.repo"] checkout --> remote["Configured upstream Git remote"] cli --> github["GitHub PR boundary"] @@ -56,7 +57,7 @@ The CLI reads downstream source, validates and temporarily mutates only the name ## Selected Solution -- `SOL-01` Add `memory-bank-cli push [--dry-run]`. Resolve the current Git root, read changed paths under `memory-bank/`, and produce an inclusion/exclusion plan before mutation. +- `SOL-01` Add `memory-bank-cli push [--dry-run]`. Resolve the current Git root, read all changed repository-relative paths, and produce an inclusion/exclusion plan from the ownership lock before mutation. - `SOL-02` Treat `memory-bank/.repo` as the only upstream checkout. Validate safe path, clean/conflict-free Git state, upstream remote/identity and non-default branch before applying a plan to a newly created branch. - `SOL-03` Create the GitHub PR only after push. On any post-preflight failure follow `SD-02`; output includes plan and PR URL, while dry-run has no mutation. @@ -77,22 +78,22 @@ The CLI reads downstream source, validates and temporarily mutates only the name ## Accepted Local Decisions -- `SD-01` A path is publishable only when safely normalized below `memory-bank/` and its current class is exactly `managed`; every other class is reported as excluded. Failed normalization/classification aborts before mutation. +- `SD-01` With an ownership lock, a safely normalized repository-relative path is publishable only when its lock entry is exactly `managed`; every other path is reported as excluded. Lockless legacy repositories retain the bounded `memory-bank/` classifier fallback. Failed normalization, lock validation or classification aborts before mutation. - `SD-02` Preflight first; use only a fresh non-default branch; after failure restore original local branch/HEAD and attempt remote deletion only for the command-created branch. Failed compensation is a diagnosed failed outcome, never a default-branch write. - `SD-03` `standard` validation requires targeted contract tests, full Go suite, vet, navigation audit and one approved live PR result; the latter is a closure gate, not a unit-test substitute. -- `SD-04` The source namespace is always downstream `memory-bank/`; before mutation `.repo` must resolve as its own Git worktree and the selected `origin/default` tree must contain exactly one real payload root, `memory-bank-template/` or legacy `memory-bank/`. The planner translates only the leading namespace and rejects missing, duplicate or symlink payload roots. +- `SD-04` The canonical upstream root is the whole `template/` tree. Its shared path model strips `template/` for init/update and prefixes `template/` for push without inspecting any child name; therefore `template/memory-bank/**` round-trips as `memory-bank/**` and every other locked path retains its suffix. Before mutation `.repo` must resolve as its own Git worktree and the selected `origin/default` tree must contain canonical `template/` or exactly one legacy payload root. Canonical root presence takes precedence over legacy project-local trees. ## Contracts | Contract ID | Connector / direction | Roles and sync boundary | Guarantees / failure / evolution semantics | | --- | --- | --- | --- | -| `CTR-01` | Filesystem: downstream `memory-bank/` → selection planner → upstream payload root | CLI reads current repository synchronously | Only exactly-`managed` normalized paths enter plan; `.lock`, `.repo` and all other classes are excluded. The source prefix translates to exactly one validated upstream `memory-bank-template/` or `memory-bank/` root; invalid topology aborts. | +| `CTR-01` | Filesystem: lock-managed downstream paths → selection planner → upstream payload root | CLI reads current repository synchronously | Only exactly-`managed` normalized paths enter the plan. Canonical paths use the shared name-agnostic `template/` mapping; `.lock`, `.repo` and all other classes are excluded. Legacy upstream roots retain their bounded `memory-bank/`-only translation. | | `CTR-02` | Local Git and remote Git: CLI → `.repo` → configured upstream | Synchronous local/remote boundary | Preflight validates repository, safe path, clean state, conflicts, remote identity and default branch; writes only fresh branch; failure runs `SD-02`. | | `CTR-03` | GitHub PR: CLI → configured upstream repository | External authenticated boundary after push | Success returns URL; failure triggers `CTR-02` compensation; dry-run never crosses boundary. | ## Invariants -- `INV-01` No selected source path is outside `memory-bank/` or outside exactly-`managed` ownership. +- `INV-01` No selected source path is absent from exactly-`managed` ownership, and no canonical destination is outside `template/`. - `INV-02` The upstream default branch is never directly checked out for write, committed to or pushed by `push`. - `INV-03` `--dry-run` mutates neither local checkout, remote Git nor GitHub. - `INV-04` Failure reports residual state and recovery action; success requires a PR URL.