diff --git a/CHANGELOG.md b/CHANGELOG.md index aa806c0..95c4f19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.ht ## [Unreleased] +### Fixed + +- Publish to the canonical upstream `template/memory-bank/` payload root, + retaining legacy-root fallback for compatible upstream repositories. +- Reject all unresolved downstream Git conflict statuses before an upstream + publication plan can mutate state. +- Reject symlinked `memory-bank/` and `.repo` checkout ancestry, and provide + corrective next steps in push preflight diagnostics. + ## [1.4.0] - 2026-07-24 ### Added diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index d5d3c47..89f0280 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -73,6 +73,19 @@ 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") { + t.Fatalf("root help does not document push: %q", stdout.String()) + } + } +} + +func TestPushHelpDocumentsDryRun(t *testing.T) { + var stdout, stderr bytes.Buffer + if exitCode := Run([]string{"push", "--help"}, "test", &stdout, &stderr); exitCode != 0 { + t.Fatalf("help exit=%d stderr=%q", exitCode, stderr.String()) + } + if !strings.Contains(stderr.String(), "Usage: memory-bank-cli push") || !strings.Contains(stderr.String(), "without mutating checkout, remotes, or GitHub") { + t.Fatalf("push help is incomplete: %q", stderr.String()) } } diff --git a/internal/push/push.go b/internal/push/push.go index d614a13..b90ac29 100644 --- a/internal/push/push.go +++ b/internal/push/push.go @@ -56,7 +56,7 @@ func Run(options Options) (Report, error) { now = time.Now } if _, err := run(options.RepoRoot, "git", "rev-parse", "--show-toplevel"); err != nil { - return Report{}, fmt.Errorf("push must run inside a Git repository: %w", err) + return Report{}, fmt.Errorf("push must run inside a Git repository; run it from a repository or pass --repo-root: %w", err) } checkout, err := safeCheckout(options.RepoRoot) if err != nil { @@ -67,7 +67,7 @@ func Run(options Options) (Report, error) { } remote, err := run(checkout, "git", "remote", "get-url", "origin") if err != nil || strings.TrimSpace(remote) == "" { - return Report{}, fmt.Errorf("upstream checkout has no valid origin remote") + return Report{}, errors.New("upstream checkout has no valid origin remote; configure memory-bank/.repo origin to the target GitHub repository") } githubRepo, err := githubRepository(strings.TrimSpace(remote)) if err != nil { @@ -75,18 +75,13 @@ func Run(options Options) (Report, error) { } if !options.DryRun { if identity, err := run(checkout, "gh", "repo", "view", githubRepo, "--json", "id"); err != nil || strings.TrimSpace(identity) == "" { - return Report{}, errors.New("upstream origin is not an accessible GitHub repository for PR creation") + return Report{}, fmt.Errorf("upstream origin %q is not accessible for PR creation; run gh auth login and verify repository access", githubRepo) } } defaultBranch, err := defaultBranch(run, checkout) if err != nil { return Report{}, err } - if !options.DryRun { - if _, err := run(checkout, "git", "fetch", "origin", defaultBranch+":refs/remotes/origin/"+defaultBranch); err != nil { - return Report{}, fmt.Errorf("refresh upstream default branch: %w", err) - } - } payloadRoot, err := selectPayloadRootAt(run, checkout, "origin/"+defaultBranch) if err != nil { return Report{}, err @@ -127,7 +122,17 @@ func Run(options Options) (Report, error) { } } if len(included) == 0 { - return report, errors.New("no managed Memory Bank changes to publish") + return report, errors.New("no managed Memory Bank changes to publish; edit a managed path or inspect exclusions with --dry-run") + } + if _, err := run(checkout, "git", "fetch", "origin", defaultBranch+":refs/remotes/origin/"+defaultBranch); err != nil { + return report, fmt.Errorf("refresh upstream default branch: %w", err) + } + refreshedPayloadRoot, err := selectPayloadRootAt(run, checkout, "origin/"+defaultBranch) + if err != nil { + return report, err + } + if refreshedPayloadRoot != payloadRoot { + return report, fmt.Errorf("upstream payload root changed from %q to %q while refreshing the default branch; retry the command", payloadRoot, refreshedPayloadRoot) } makeBranch := options.BranchName if makeBranch == nil { @@ -228,15 +233,24 @@ func Run(options Options) (Report, error) { } func safeCheckout(root string) (string, error) { - path := filepath.Join(root, "memory-bank", ".repo") - info, err := os.Lstat(path) - if err != nil { - return "", fmt.Errorf("inspect upstream checkout: %w", err) - } - if info.Mode()&os.ModeSymlink != 0 || !info.IsDir() { - return "", errors.New("memory-bank/.repo must be a real directory, not a symlink") + memoryBank := filepath.Join(root, "memory-bank") + checkout := filepath.Join(memoryBank, ".repo") + for _, candidate := range []struct { + path string + label string + }{ + {path: memoryBank, label: "memory-bank"}, + {path: checkout, label: "memory-bank/.repo"}, + } { + info, err := os.Lstat(candidate.path) + if err != nil { + return "", fmt.Errorf("inspect %s: %w; clone the upstream repository into memory-bank/.repo", candidate.label, err) + } + if info.Mode()&os.ModeSymlink != 0 || !info.IsDir() { + return "", fmt.Errorf("%s must be a real directory, not a symlink; replace it with a local upstream checkout", candidate.label) + } } - return path, nil + return checkout, nil } func githubRepository(remote string) (string, error) { @@ -246,11 +260,11 @@ func githubRepository(remote string) (string, error) { } else if strings.HasPrefix(value, "https://github.com/") { value = strings.TrimPrefix(value, "https://github.com/") } else { - return "", fmt.Errorf("upstream origin must be a GitHub repository: %q", remote) + return "", fmt.Errorf("upstream origin must be a GitHub repository: %q; set memory-bank/.repo origin to a GitHub SSH or HTTPS URL", remote) } parts := strings.Split(value, "/") if len(parts) != 2 || parts[0] == "" || parts[1] == "" { - return "", fmt.Errorf("invalid GitHub origin: %q", remote) + return "", fmt.Errorf("invalid GitHub origin: %q; use an owner/repository GitHub URL", remote) } return value, nil } @@ -274,19 +288,19 @@ func clean(run func(string, string, ...string) (string, error), dir string) erro if filepath.Clean(absDir) != filepath.Clean(absTop) { return fmt.Errorf("memory-bank/.repo must be its own Git worktree (got %q, want %q)", absTop, absDir) } - status, err := run(dir, "git", "status", "--porcelain") + conflicts, err := run(dir, "git", "diff", "--name-only", "--diff-filter=U") if err != nil { return err } - if strings.TrimSpace(status) != "" { - return errors.New("upstream checkout is dirty; commit, stash, or discard its changes first") + if strings.TrimSpace(conflicts) != "" { + return errors.New("upstream checkout has unresolved conflicts; resolve them before running push") } - conflicts, err := run(dir, "git", "diff", "--name-only", "--diff-filter=U") + status, err := run(dir, "git", "status", "--porcelain") if err != nil { return err } - if strings.TrimSpace(conflicts) != "" { - return errors.New("upstream checkout has unresolved conflicts") + if strings.TrimSpace(status) != "" { + return errors.New("upstream checkout is dirty; commit, stash, or discard its changes first") } return nil } @@ -294,19 +308,32 @@ func clean(run func(string, string, ...string) (string, error), dir string) erro func defaultBranch(run func(string, string, ...string) (string, error), checkout string) (string, error) { ref, err := run(checkout, "git", "symbolic-ref", "--short", "refs/remotes/origin/HEAD") if err != nil { - return "", fmt.Errorf("resolve upstream default branch: %w", err) + return "", fmt.Errorf("resolve upstream default branch: %w; run git -C memory-bank/.repo remote set-head origin --auto", err) } branch := strings.TrimPrefix(strings.TrimSpace(ref), "origin/") if branch == "" || branch == ref { - return "", errors.New("upstream origin/HEAD does not name a default branch") + return "", errors.New("upstream origin/HEAD does not name a default branch; run git -C memory-bank/.repo remote set-head origin --auto") } if _, err := run(checkout, "git", "rev-parse", "--verify", "origin/"+branch); err != nil { - return "", fmt.Errorf("default branch %q is not available locally: %w", branch, err) + return "", fmt.Errorf("default branch %q is not available locally: %w; fetch origin and retry", branch, err) } return branch, nil } func selectPayloadRoot(checkout string) (string, error) { + canonical := filepath.Join(checkout, "template", "memory-bank") + if info, err := os.Lstat(canonical); err == nil { + if !info.IsDir() || info.Mode()&os.ModeSymlink != 0 { + return "", errors.New("upstream payload root \"template/memory-bank\" must be a real directory") + } + template, err := os.Lstat(filepath.Join(checkout, "template")) + if err != nil || !template.IsDir() || template.Mode()&os.ModeSymlink != 0 { + return "", errors.New("upstream payload root \"template/memory-bank\" has an unsafe parent directory") + } + return "template/memory-bank", nil + } else if !errors.Is(err, os.ErrNotExist) { + return "", err + } var roots []string for _, candidate := range []string{"memory-bank-template", "memory-bank"} { info, err := os.Lstat(filepath.Join(checkout, candidate)) @@ -322,12 +349,20 @@ 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 template/memory-bank or exactly one legacy payload root (memory-bank-template or memory-bank), found %v; check out the upstream default branch and fix its payload layout", roots) } return roots[0], nil } func selectPayloadRootAt(run func(string, string, ...string) (string, error), checkout, ref string) (string, error) { + canonical := "template/memory-bank" + out, err := run(checkout, "git", "ls-tree", "-d", "--name-only", ref, "--", canonical) + if err != nil { + return "", fmt.Errorf("inspect upstream payload root %q: %w", canonical, err) + } + if strings.TrimSpace(out) == canonical { + return canonical, nil + } var roots []string for _, candidate := range []string{"memory-bank-template", "memory-bank"} { out, err := run(checkout, "git", "ls-tree", "-d", "--name-only", ref, "--", candidate) @@ -339,7 +374,7 @@ 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 template/memory-bank or exactly one legacy payload root (memory-bank-template or memory-bank), found %v; verify the upstream repository payload layout", roots) } return roots[0], nil } @@ -357,8 +392,8 @@ func changedPaths(run func(string, string, ...string) (string, error), root stri continue } status, paths := line[:2], line[3:] - if strings.Contains(status, "U") { - return nil, fmt.Errorf("downstream path has unresolved Git conflict: %q", paths) + if unmergedStatus(status) { + return nil, fmt.Errorf("downstream path has unresolved Git conflict: %q; resolve the conflict before running push", paths) } if strings.Contains(status, "R") { if index+1 >= len(records) || records[index+1] == "" { @@ -390,6 +425,15 @@ func changedPaths(run func(string, string, ...string) (string, error), root stri return paths, nil } +func unmergedStatus(status string) bool { + switch status { + case "DD", "AU", "UD", "UA", "DU", "AA", "UU": + return true + default: + return false + } +} + func branchName(now time.Time) (string, error) { bytes := make([]byte, 6) if _, err := rand.Read(bytes); err != nil { diff --git a/internal/push/push_test.go b/internal/push/push_test.go index 75a7d35..3b31aec 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,8 @@ 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 -- memory-bank-template": - return "memory-bank-template", nil + case "git ls-tree -d --name-only origin/main -- template/memory-bank": + return "template/memory-bank", nil case "git ls-tree -d --name-only origin/main -- memory-bank": return "", nil case "git status --porcelain=v1 -z --untracked-files=all -- memory-bank": @@ -71,7 +71,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 +95,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 +109,8 @@ 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 -- memory-bank-template": - return "memory-bank-template", nil + case "git ls-tree -d --name-only origin/main -- template/memory-bank": + return "template/memory-bank", nil case "git ls-tree -d --name-only origin/main -- memory-bank": return "", nil case "git status --porcelain=v1 -z --untracked-files=all -- memory-bank": @@ -137,6 +137,62 @@ func TestRunCompensatesRemoteBranchWhenPRCreationFails(t *testing.T) { } } +func TestRunRejectsDownstreamConflictBeforeRefresh(t *testing.T) { + root := pushFixture(t) + fetched := false + run := func(dir, name string, args ...string) (string, error) { + call := name + " " + strings.Join(args, " ") + switch call { + case "git rev-parse --show-toplevel": + return dir, nil + case "git rev-parse --is-inside-work-tree", "git status --porcelain", "git diff --name-only --diff-filter=U", "git rev-parse --verify origin/main": + return "", nil + case "git remote get-url origin": + return "https://github.com/example/upstream.git", nil + case "gh repo view example/upstream --json id": + return "{\"id\":\"R_1\"}", nil + case "git symbolic-ref --short refs/remotes/origin/HEAD": + return "origin/main", nil + case "git ls-tree -d --name-only origin/main -- template/memory-bank": + return "template/memory-bank", nil + case "git branch --show-current": + return "main", nil + case "git rev-parse HEAD": + return "abc123", nil + case "git status --porcelain=v1 -z --untracked-files=all -- memory-bank": + return "AA memory-bank/dna/rule.md\x00", nil + case "git fetch origin main:refs/remotes/origin/main": + fetched = true + return "", nil + default: + return "", errors.New("unexpected command: " + call) + } + } + _, err := Run(Options{RepoRoot: root, Run: run}) + if err == nil || !strings.Contains(err.Error(), "unresolved Git conflict") { + t.Fatalf("want conflict error, got %v", err) + } + if fetched { + t.Fatal("preflight conflict refreshed the upstream tracking ref") + } +} + +func TestSelectPayloadRootPrefersCanonicalRoot(t *testing.T) { + checkout := t.TempDir() + for _, root := range []string{"template/memory-bank", "memory-bank-template", "memory-bank"} { + if err := os.MkdirAll(filepath.Join(checkout, filepath.FromSlash(root)), 0o755); err != nil { + t.Fatal(err) + } + } + root, err := selectPayloadRoot(checkout) + if err != nil { + t.Fatal(err) + } + if root != "template/memory-bank" { + t.Fatalf("got %q, want canonical root", root) + } +} + func pushFixture(t *testing.T) string { t.Helper() root := t.TempDir() @@ -146,7 +202,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 { @@ -181,16 +237,19 @@ func TestDryRunIncludesOnlyManagedPaths(t *testing.T) { } git(t, upstream, "init", "--quiet") git(t, upstream, "remote", "add", "origin", "https://github.com/example/upstream.git") - if err := os.MkdirAll(filepath.Join(upstream, "memory-bank-template"), 0o755); err != nil { + if err := os.MkdirAll(filepath.Join(upstream, "template", "memory-bank"), 0o755); err != nil { t.Fatal(err) } - if err := os.WriteFile(filepath.Join(upstream, "memory-bank-template", ".keep"), []byte("base\n"), 0o644); err != nil { + if err := os.WriteFile(filepath.Join(upstream, "template", "memory-bank", ".keep"), []byte("base\n"), 0o644); err != nil { t.Fatal(err) } git(t, upstream, "add", ".") git(t, upstream, "-c", "user.name=Test", "-c", "user.email=test@example.invalid", "commit", "--quiet", "-m", "base") git(t, upstream, "update-ref", "refs/remotes/origin/master", "HEAD") git(t, upstream, "symbolic-ref", "refs/remotes/origin/HEAD", "refs/remotes/origin/master") + headBefore := git(t, upstream, "rev-parse", "HEAD") + branchBefore := git(t, upstream, "branch", "--show-current") + statusBefore := git(t, upstream, "status", "--porcelain") if err := os.WriteFile(filepath.Join(root, "memory-bank", "dna", "rule.md"), []byte("changed\n"), 0o644); err != nil { t.Fatal(err) } @@ -217,6 +276,15 @@ func TestDryRunIncludesOnlyManagedPaths(t *testing.T) { if _, err := os.Stat(filepath.Join(upstream, ".git")); err != nil { t.Fatalf("dry run changed checkout: %v", err) } + if headAfter := git(t, upstream, "rev-parse", "HEAD"); headAfter != headBefore { + t.Fatalf("dry run changed upstream HEAD from %s to %s", headBefore, headAfter) + } + if branchAfter := git(t, upstream, "branch", "--show-current"); branchAfter != branchBefore { + t.Fatalf("dry run changed upstream branch from %q to %q", branchBefore, branchAfter) + } + if statusAfter := git(t, upstream, "status", "--porcelain"); statusAfter != statusBefore { + t.Fatalf("dry run changed upstream status from %q to %q", statusBefore, statusAfter) + } } func TestRejectsDirtyUpstreamCheckout(t *testing.T) { @@ -272,6 +340,94 @@ func TestChangedPathsRepresentsDeletionAndRename(t *testing.T) { } } +func TestChangedPathsRejectsEveryUnmergedStatus(t *testing.T) { + for _, status := range []string{"DD", "AU", "UD", "UA", "DU", "AA", "UU"} { + t.Run(status, func(t *testing.T) { + _, err := changedPaths(func(_ string, _ string, _ ...string) (string, error) { + return status + " memory-bank/dna/conflict.md\x00", nil + }, "unused") + if err == nil || !strings.Contains(err.Error(), "resolve the conflict") { + t.Fatalf("status %s should return actionable conflict error, got %v", status, err) + } + }) + } +} + +func TestSafeCheckoutRejectsSymlinkedMemoryBankParent(t *testing.T) { + root, outside := t.TempDir(), t.TempDir() + if err := os.MkdirAll(filepath.Join(outside, ".repo"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(outside, filepath.Join(root, "memory-bank")); err != nil { + t.Skipf("symlink unsupported: %v", err) + } + _, err := safeCheckout(root) + if err == nil || !strings.Contains(err.Error(), "memory-bank must be a real directory") || !strings.Contains(err.Error(), "replace it") { + t.Fatalf("want actionable symlink-parent rejection, got %v", err) + } +} + +func TestSafeCheckoutRejectsSymlinkedRepo(t *testing.T) { + root, outside := t.TempDir(), t.TempDir() + if err := os.Mkdir(filepath.Join(root, "memory-bank"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(outside, filepath.Join(root, "memory-bank", ".repo")); err != nil { + t.Skipf("symlink unsupported: %v", err) + } + _, err := safeCheckout(root) + if err == nil || !strings.Contains(err.Error(), "memory-bank/.repo must be a real directory") || !strings.Contains(err.Error(), "replace it") { + t.Fatalf("want actionable symlink-checkout rejection, got %v", err) + } +} + +func TestCleanReportsUpstreamConflictBeforeDirtyState(t *testing.T) { + checkout := t.TempDir() + run := func(_ string, name string, args ...string) (string, error) { + call := name + " " + strings.Join(args, " ") + switch call { + case "git rev-parse --is-inside-work-tree": + return "true", nil + case "git rev-parse --show-toplevel": + return checkout, nil + case "git diff --name-only --diff-filter=U": + return "memory-bank/dna/conflict.md", nil + case "git status --porcelain": + return "UU memory-bank/dna/conflict.md", nil + default: + return "", errors.New("unexpected command: " + call) + } + } + err := clean(run, checkout) + if err == nil || !strings.Contains(err.Error(), "unresolved conflicts") || !strings.Contains(err.Error(), "resolve them") { + t.Fatalf("want actionable upstream-conflict error, got %v", err) + } +} + +func TestGitHubRepositoryAcceptsDefaultAndCustomUpstreams(t *testing.T) { + for _, test := range []struct { + remote string + want string + }{ + {remote: "git@github.com:dapi/memory-bank.git", want: "dapi/memory-bank"}, + {remote: "https://github.com/example/custom-bank.git", want: "example/custom-bank"}, + } { + t.Run(test.want, func(t *testing.T) { + got, err := githubRepository(test.remote) + if err != nil || got != test.want { + t.Fatalf("githubRepository(%q) = %q, %v; want %q", test.remote, got, err, test.want) + } + }) + } +} + +func TestGitHubRepositoryRejectsInvalidRemoteWithNextStep(t *testing.T) { + _, err := githubRepository("ssh://git@example.invalid/bank.git") + if err == nil || !strings.Contains(err.Error(), "set memory-bank/.repo origin") { + t.Fatalf("want actionable invalid-remote error, got %v", err) + } +} + func TestCopyRegularRejectsSymlinkSource(t *testing.T) { root := t.TempDir() sourceRoot, destinationRoot := filepath.Join(root, "source"), filepath.Join(root, "destination") diff --git a/memory-bank/features/FT-023-push-upstream-publication/README.md b/memory-bank/features/FT-023-push-upstream-publication/README.md index 386fd1a..9933bbd 100644 --- a/memory-bank/features/FT-023-push-upstream-publication/README.md +++ b/memory-bank/features/FT-023-push-upstream-publication/README.md @@ -20,6 +20,7 @@ audience: humans_and_agents - [brief.md](brief.md) — canonical problem space, validation-profile decision и verify contract. - [design.md](design.md) — selected publication, transaction and Git/GitHub boundary design. -- [implementation-plan.md](implementation-plan.md) — grounded execution, tests and live-validation approval gate. +- [implementation-plan.md](implementation-plan.md) — archived execution and + validation record; all checkpoints are closed. - [decision-log.md](decision-log.md) — provenance и FPF-разбор blocking decisions; не владеет требованиями или selected solution. - [feature-review-report.md](feature-review-report.md) — результат bounded review–improve. diff --git a/memory-bank/features/FT-023-push-upstream-publication/brief.md b/memory-bank/features/FT-023-push-upstream-publication/brief.md index 37e5275..444436a 100644 --- a/memory-bank/features/FT-023-push-upstream-publication/brief.md +++ b/memory-bank/features/FT-023-push-upstream-publication/brief.md @@ -6,9 +6,10 @@ purpose: "Canonical brief для публикации upstream-пригодны derived_from: - ../../flows/feature.md - ../../engineering/validation-profiles.md + - ../../use-cases/UC-004-publish-managed-changes-upstream.md - "https://github.com/dapi/memory-bank-cli/issues/23" status: active -delivery_status: planned +delivery_status: done audience: humans_and_agents must_not_define: - implementation_sequence @@ -115,8 +116,30 @@ Issue #23 задаёт качественные acceptance criteria, но не ### Evidence -- `EVID-01` Targeted test output and approved live upstream PR URL. -- `EVID-02` Targeted preflight/failure test output. -- `EVID-03` Targeted selection-fixture test output. -- `EVID-04` Targeted transaction/recovery test output and review record for any externally observable recovery. -- `EVID-05` Targeted dry-run test output. +- `EVID-01` **PASS** — `TestRunCreatesBranchCopiesManagedFileAndReturnsPR`, + default/custom-upstream contract tests, and the controlled live + [dapi/memory-bank#78](https://github.com/dapi/memory-bank/pull/78). The live + PR targeted `main` through a fresh branch and contained one managed path at + `template/memory-bank/dna/push-live-validation.md`. +- `EVID-02` **PASS** — `TestRejectsDirtyUpstreamCheckout`, + `TestSafeCheckoutRejectsSymlinkedMemoryBankParent`, + `TestSafeCheckoutRejectsSymlinkedRepo`, + `TestCleanReportsUpstreamConflictBeforeDirtyState`, invalid-remote coverage, + and the complete unmerged-status table pass in + [Validate run 30116321734](https://github.com/dapi/memory-bank-cli/actions/runs/30116321734). +- `EVID-03` **PASS** — `TestDryRunIncludesOnlyManagedPaths` proves managed + inclusion and adapted exclusion; the live carrier included only + `template/memory-bank/dna/push-live-validation.md`. +- `EVID-04` **PASS** — `TestRunCompensatesRemoteBranchWhenPRCreationFails` + covers injected PR failure and compensation. Live PR #78 left upstream + `main` at `9a4463cf8ee06860a0a7238cb283337df0ae496e`, was closed without + merge, and its temporary branch was deleted. +- `EVID-05` **PASS** — targeted dry-run coverage asserts unchanged upstream + HEAD, branch and status; the live dry run also left the remote default SHA + unchanged. + +The full required CI profile passed in the +[completion PR #34](https://github.com/dapi/memory-bank-cli/pull/34): +[Validate](https://github.com/dapi/memory-bank-cli/actions/runs/30116321734), +[local E2E](https://github.com/dapi/memory-bank-cli/actions/runs/30116321739), +and [stable downstream smoke](https://github.com/dapi/memory-bank-cli/actions/runs/30116321679). 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..2091899 100644 --- a/memory-bank/features/FT-023-push-upstream-publication/design.md +++ b/memory-bank/features/FT-023-push-upstream-publication/design.md @@ -80,13 +80,13 @@ The CLI reads downstream source, validates and temporarily mutates only the name - `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-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 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. ## 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: 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-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. | @@ -111,13 +111,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 | `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` | +| 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` | | 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 | `CHK-01`, full suite | +| Migration / evolution safety | yes | CLI regression and non-default-upstream tests | passed; `EVID-01` and completion PR #34 | ## ADR / External Design Dependencies diff --git a/memory-bank/features/FT-023-push-upstream-publication/implementation-plan.md b/memory-bank/features/FT-023-push-upstream-publication/implementation-plan.md index 1806d2d..9ce0757 100644 --- a/memory-bank/features/FT-023-push-upstream-publication/implementation-plan.md +++ b/memory-bank/features/FT-023-push-upstream-publication/implementation-plan.md @@ -6,7 +6,7 @@ purpose: "Execution and verification record for FT-023; refines canonical brief/ derived_from: - brief.md - design.md -status: active +status: archived audience: humans_and_agents must_not_define: - ft_023_scope @@ -30,10 +30,10 @@ must_not_define: | Surface | Canonical refs | Automated coverage | Command | Manual-only gap | | --- | --- | --- | --- | --- | -| selection and dry-run | `SD-01`, `CTR-01`, `INV-01`, `INV-03` | managed inclusion and adapted exclusion | `go test ./internal/push` | none | -| preflight | `CTR-02`, `FM-01` | dirty nested checkout rejection | `go test ./internal/push` | real remote/GitHub boundary | -| CLI regression | `SOL-01`, `SOL-03` | command integration and root help | `go test ./internal/cli` | live PR creation | -| repository regression | `SD-03` | full suite, vet, navigation | `go test ./...`; `go vet ./...`; `go run ./cmd/memory-bank-cli lint --repo-root .` | approved live PR evidence | +| selection and dry-run | `SD-01`, `CTR-01`, `INV-01`, `INV-03` | managed inclusion, adapted exclusion and state immutability | `go test ./internal/push` | none | +| preflight | `CTR-02`, `FM-01` | dirty, unsafe, conflicted and invalid-remote rejection | `go test ./internal/push` | none | +| CLI regression | `SOL-01`, `SOL-03` | command integration and root/push help | `go test ./internal/cli` | none | +| repository regression | `SD-03` | full suite, vet, navigation and approved live PR | `go test -count=1 -race ./...`; `go vet ./...`; `go run ./cmd/memory-bank-cli lint --repo-root .` | none; live carrier is dapi/memory-bank#78 | ## Open Questions / Ambiguities @@ -65,13 +65,15 @@ must_not_define: | Checkpoint ID | Condition | | --- | --- | -| `CP-01` | managed-only and dirty-checkout tests pass. | -| `CP-02` | full Go suite, vet and navigation audit pass. | -| `CP-03` | approved live PR URL or exact recovery evidence is recorded. | +| `CP-01` | **PASS** — managed-only, dry-run and preflight failure tests pass. | +| `CP-02` | **PASS** — full race suite, vet, navigation audit and PR CI pass. | +| `CP-03` | **PASS** — live [dapi/memory-bank#78](https://github.com/dapi/memory-bank/pull/78) confirmed the canonical `template/memory-bank/` target and default-branch preservation; details are recorded in `brief.md`. | ## Approval Gate -`AG-01`: a real `push` creates an external upstream branch and PR; it requires upstream-owner approval and credentials. This PR does not claim `EVID-01`/`EVID-04` live evidence. +`AG-01`: the repository owner's completion request authorized controlled live +validation. [dapi/memory-bank#78](https://github.com/dapi/memory-bank/pull/78) +was created against `main`, verified, closed without merge and cleaned up. ## Stop Conditions / Fallback @@ -82,4 +84,11 @@ must_not_define: ## Plan-local Evidence -- `EVID-06`: local implementation verification recorded in this PR. +- `EVID-06`: local and CI implementation verification is recorded in + [completion PR #34](https://github.com/dapi/memory-bank-cli/pull/34). + +## Archive Note + +All checkpoints and the manual approval gate are closed. Canonical acceptance +evidence remains in `brief.md`; this execution plan is archived with +`delivery_status: done`. diff --git a/memory-bank/features/README.md b/memory-bank/features/README.md index 48d0c02..bf2df21 100644 --- a/memory-bank/features/README.md +++ b/memory-bank/features/README.md @@ -24,7 +24,7 @@ audience: humans_and_agents - [FT-006: Opt-in GitHub Workflow Adapter](FT-006-github-workflow-adapter/README.md) — active delivery package for issue #6's opt-in, marker-owned GitHub workflow adapter. - [FT-014: Source Payload Path Translation](FT-014-source-payload-path-translation/README.md) — planned package for issue #14's source `memory-bank-template/` to downstream `memory-bank/` translation and release handoff. - [FT-021: Local Init/Update E2E](FT-021-local-init-update-e2e/README.md) — planned package for issue #21's hermetic CLI/Git lifecycle coverage, required local merge gate and pre-publish release-binary validation. -- [FT-023: Push Upstream Publication](FT-023-push-upstream-publication/README.md) — bootstrap package for issue #23's safe publication of upstream-suitable downstream changes through a PR; blocked pending selection, recovery and validation decisions. +- [FT-023: Push Upstream Publication](FT-023-push-upstream-publication/README.md) — completed package for issue #23's managed-only, failure-safe upstream publication through a dedicated branch and PR. - [FT-026: Ignore Project-Local Source Root](FT-026-ignore-project-local-source-root/README.md) — in-progress package for issue #26's target-template selection alongside a locked project-local copy. ## Rules diff --git a/memory-bank/use-cases/README.md b/memory-bank/use-cases/README.md index 8835254..04ff65a 100644 --- a/memory-bank/use-cases/README.md +++ b/memory-bank/use-cases/README.md @@ -17,3 +17,4 @@ status: active | `UC-001` | [Adopt a template](UC-001-adopt-template.md) | A maintainer creates an ownership-tracked Memory Bank from a pinned source. | active | Repository maintainer | none | existing CLI | | `UC-002` | [Safely update a template](UC-002-update-template.md) | A maintainer previews or applies a conflict-aware update without overwriting protected content. | active | Repository maintainer | none | existing CLI | | `UC-003` | [Audit documentation](UC-003-audit-documentation.md) | A contributor or automation obtains navigation/governance diagnostic findings. | active | Contributor / automation | none | existing CLI | +| `UC-004` | [Publish managed changes upstream](UC-004-publish-managed-changes-upstream.md) | A maintainer previews or publishes managed downstream changes through a safe upstream PR. | active | Repository maintainer | none | `FT-023` | diff --git a/memory-bank/use-cases/UC-004-publish-managed-changes-upstream.md b/memory-bank/use-cases/UC-004-publish-managed-changes-upstream.md new file mode 100644 index 0000000..12539b2 --- /dev/null +++ b/memory-bank/use-cases/UC-004-publish-managed-changes-upstream.md @@ -0,0 +1,87 @@ +--- +title: "UC-004: Publish Managed Changes Upstream" +doc_kind: use_case +doc_function: canonical +purpose: "Canonical owner of the stable managed-change upstream publication scenario." +derived_from: + - ../flows/use-case.md + - ../product/context.md + - ../domain/rules.md +status: active +audience: humans_and_agents +must_not_define: + - implementation_sequence + - architecture_decision + - feature_level_test_matrix +--- + +# UC-004: Publish Managed Changes Upstream + +## Goal + +Propose reusable downstream Memory Bank changes to the configured upstream +repository without publishing project-specific state or writing directly to +the upstream default branch. + +## Primary Actor + +Repository maintainer. + +## Trigger + +The maintainer has validated reusable changes in a downstream Memory Bank and +wants to propose them upstream. + +## Preconditions + +- The current directory or `--repo-root` identifies a Git repository. +- `memory-bank/.repo` is a real, clean, conflict-free checkout with an + accessible GitHub `origin` and a resolvable default branch. +- The downstream repository contains changed managed Memory Bank paths. + +## Main Flow + +1. The actor runs `memory-bank-cli push --dry-run` and reviews every inclusion + and exclusion. +2. The actor runs `memory-bank-cli push`. +3. The CLI revalidates the checkout and upstream identity, creates a fresh + branch from the upstream default branch, copies only managed changes, + commits and pushes that branch, and creates a GitHub pull request. +4. The CLI reports the decisions and created pull-request URL. + +## Alternate Flows / Exceptions + +- `ALT-01` A dry run reports the plan without changing the checkout, remotes or + GitHub. +- `EX-01` Unsafe paths, dirty state, unresolved conflicts, an invalid remote, + ambiguous paths or an empty managed set stop before publication with a + corrective next step. +- `EX-02` A post-mutation failure restores the original local checkout and + attempts bounded cleanup of only the command-created remote branch; any + residual state is reported. + +## Postconditions + +- On success, an upstream PR targets the default branch from a dedicated + non-default branch and contains only managed Memory Bank paths. +- On dry-run or preflight failure, no working-tree files, upstream branch or + commit, remote branch, or GitHub PR is created. A non-dry validation may + refresh the local `origin/` tracking ref. +- The upstream default branch is never a direct push target. + +## Business Rules + +- `BR-01` Only paths classified as `managed` are publishable by this flow. +- `BR-02` Project-specific, adapted, generated, lock/state, `.repo` and unknown + paths are excluded. +- `BR-03` Publication through a dedicated PR is mandatory; automatic merge and + direct default-branch push are outside this flow. + +## Traceability + +| Upstream / Downstream | References | +| --- | --- | +| PRD | none | +| Features | `FT-023` | +| ADR | none | +| Runbooks / Ops | none |