diff --git a/README.md b/README.md index b54d476..df64563 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 111347d..cb7f5f3 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/cli/cli_test.go b/internal/cli/cli_test.go index c4daa76..167865b 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -73,7 +73,7 @@ func TestRootHelpAndVersion(t *testing.T) { if !strings.Contains(stdout.String(), test.want) { t.Fatalf("unexpected stdout for %v: %q", test.arguments, stdout.String()) } - if test.arguments[0] == "--help" && !strings.Contains(stdout.String(), "push Publish managed Memory Bank changes upstream through a PR") { + if test.arguments[0] == "--help" && !strings.Contains(stdout.String(), "push Publish locked canonical template changes upstream through a PR") { t.Fatalf("root help does not document push: %q", stdout.String()) } } 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..9108ed2 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 || path.IsAbs(filePath) || 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 == "." || isGitMetadataPath(filePath) { return Lock{}, false, "", fmt.Errorf("invalid path %q in %s", filePath, LockFileName) } switch file.Ownership { @@ -91,6 +91,15 @@ func readLockSnapshot(repo pinnedRepo) (Lock, bool, string, error) { return lock, true, lockDigest, nil } +func isGitMetadataPath(filePath string) bool { + for _, component := range strings.Split(filePath, "/") { + if strings.EqualFold(component, ".git") { + return true + } + } + return false +} + func marshalLock(lock Lock) ([]byte, error) { data, err := json.MarshalIndent(lock, "", " ") if err != nil { 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..0436bc8 100644 --- a/internal/ownership/source_test.go +++ b/internal/ownership/source_test.go @@ -238,6 +238,36 @@ func TestCanonicalTemplateIncludesAllTrackedFiles(t *testing.T) { } } +func TestCanonicalTemplateRejectsGitMetadataPath(t *testing.T) { + source, repo := t.TempDir(), t.TempDir() + write(t, source, "template/.git/hooks/post-commit", "#!/bin/sh\n") + _, err := Init(Options{ + RepoRoot: repo, SourceRoot: source, TemplateVersion: "v1", SourceRef: strings.Repeat("a", 40), + verifySource: func(string, string) error { return nil }, + }) + if err == nil || !strings.Contains(err.Error(), "reserved metadata path: .git/hooks/post-commit") { + t.Fatalf("Init() error = %v, want reserved Git metadata path", err) + } + if _, err := os.Stat(filepath.Join(repo, ".git", "hooks", "post-commit")); !os.IsNotExist(err) { + t.Fatalf("Git metadata was written: %v", err) + } +} + +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..b6ce887 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 { @@ -411,8 +405,10 @@ func buildPlan(repo pinnedRepo, source map[string]payload, old Lock, hasLock boo if _, err := inspectRepoRoot(repo.root, repo.info); err != nil { return nil, nil, Lock{}, err } - if _, exists := source[LockFileName]; exists { - return nil, nil, Lock{}, fmt.Errorf("template source contains reserved metadata path: %s", LockFileName) + for sourcePath := range source { + if sourcePath == LockFileName || isGitMetadataPath(sourcePath) { + return nil, nil, Lock{}, fmt.Errorf("template source contains reserved metadata path: %s", sourcePath) + } } next := Lock{Files: make(map[string]File)} var removalMutations []mutation @@ -543,6 +539,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 2091899..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 canonical `template/memory-bank/` or exactly one legacy payload root, `memory-bank-template/` or `memory-bank/`. The canonical root wins when present; the planner translates only the leading namespace and rejects missing, duplicate or symlink legacy 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 canonical `template/memory-bank/`, with `memory-bank-template/` or `memory-bank/` as validated legacy fallbacks; 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. @@ -111,13 +112,13 @@ The CLI reads downstream source, validates and temporarily mutates only the name | Analysis | Required | Method | Result / evidence | | --- | --- | --- | --- | -| Contract compatibility | yes | FPF consequence review plus contract tests | passed; `decision-log.md` and `brief.md` `EVID-01`–`EVID-05` | -| State / transition completeness | yes | Walk through preflight → branch → commit → push → PR → compensate | passed; `SD-02`, `FM-*`, `EVID-04` | -| Failure propagation | yes | Failure-mode review and injected-failure tests | passed; `FM-01`–`FM-03`, `EVID-02`, `EVID-04` | -| Concurrency / ordering | yes | Pin/revalidate preconditions and sequential Git-command tests | passed; pre-mutation rejection and `EVID-04` | -| Security boundaries | yes | Path/remote validation review and negative tests | passed; `CTR-02`, `FM-01`, `EVID-02` | +| Contract compatibility | yes | FPF consequence review plus contract tests | `decision-log.md`; `CHK-01`–`CHK-05` evidence required before closure | +| State / transition completeness | yes | Walk through preflight → branch → commit → push → PR → compensate | all outcomes recorded in `SD-02`, `FM-*`; validate with `CHK-04` | +| Failure propagation | yes | Failure-mode review and injected-failure tests | `FM-01`–`FM-03`, `CHK-02`, `CHK-04` | +| Concurrency / ordering | yes | Pin/revalidate preconditions and sequential Git-command tests | reject state changes before mutation; `CHK-04` | +| Security boundaries | yes | Path/remote validation review and negative tests | `CTR-02`, `FM-01`, `CHK-02` | | Capacity / latency | no | One bounded operator-triggered Git operation; no new service/load path | N/A | -| Migration / evolution safety | yes | CLI regression and non-default-upstream tests | passed; `EVID-01` and completion PR #34 | +| Migration / evolution safety | yes | CLI regression and non-default-upstream tests | `CHK-01`, full suite | ## ADR / External Design Dependencies