diff --git a/README.md b/README.md index df64563..0546e34 100644 --- a/README.md +++ b/README.md @@ -40,6 +40,22 @@ memory-bank-cli --version See [CHANGELOG.md](CHANGELOG.md) for release notes. +## Repair a missing ownership lock + +`doctor` is read-only by default. If it reports `template.identity_missing`, +preview the same safe adoption plan used by `init` with pinned template +provenance: + +```sh +memory-bank-cli doctor --fix --dry-run \ + --source \ + --template-version \ + --source-ref +``` + +Rerun without `--dry-run` to create `memory-bank/.lock`, then commit that lock +before running `update`. The repair never replaces an existing lock. + ## Upgrade Install the desired newer semantic version with the same command: diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 261ee47..9e38b68 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -308,6 +308,11 @@ func runDoctor(arguments []string, stdout, stderr io.Writer) int { profileArgument := flags.String("profile", "auto", "diagnostic profile: auto, template, or downstream") scopeRootArgument := flags.String("scope-root", defaultScopeRoot, "repository-relative Memory Bank directory to diagnose") maxDepth := flags.Int("max-depth", defaultMaxDepth, "maximum navigation depth before a warning") + fix := flags.Bool("fix", false, "repair supported findings (currently template.identity_missing)") + dryRun := flags.Bool("dry-run", false, "show the complete repair mutation plan without applying it (requires --fix)") + 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 for a repair") + sourceRef := flags.String("source-ref", "", "full commit SHA matching the repair source checkout HEAD") jsonOutput := addJSONOutputFlag(flags) if err := flags.Parse(arguments); err != nil { if errors.Is(err, flag.ErrHelp) { @@ -323,6 +328,19 @@ func runDoctor(arguments []string, stdout, stderr io.Writer) int { fmt.Fprintln(stderr, "memory-bank-cli doctor: --max-depth must be greater than or equal to 0") return exitUsage } + explicitSource := *sourceRootArgument != "" || *templateVersion != "" || *sourceRef != "" + if explicitSource && (*sourceRootArgument == "" || *templateVersion == "" || *sourceRef == "") { + fmt.Fprintln(stderr, "memory-bank-cli doctor: --source, --template-version, and --source-ref must be provided together") + return exitUsage + } + if *dryRun && !*fix { + fmt.Fprintln(stderr, "memory-bank-cli doctor: --dry-run requires --fix") + return exitUsage + } + if explicitSource && !*fix { + fmt.Fprintln(stderr, "memory-bank-cli doctor: --source, --template-version, and --source-ref require --fix") + return exitUsage + } profile, err := doctor.NormalizeProfile(*profileArgument) if err != nil { fmt.Fprintln(stderr, err) @@ -353,6 +371,33 @@ func runDoctor(arguments []string, stdout, stderr io.Writer) int { fmt.Fprintln(stderr, err) return exitFailure } + if *fix && hasDoctorFinding(report, "template.identity_missing") { + if !explicitSource { + fmt.Fprintln(stderr, "memory-bank-cli doctor --fix: template.identity_missing requires --source, --template-version, and --source-ref") + return exitUsage + } + sourceRoot, err := filepath.Abs(*sourceRootArgument) + if err != nil { + fmt.Fprintln(stderr, err) + return exitFailure + } + plan, err := ownership.Init(ownership.Options{RepoRoot: repoRoot, SourceRoot: sourceRoot, TemplateVersion: *templateVersion, SourceRef: *sourceRef, DryRun: *dryRun, AgentFile: *agentFile}) + if err != nil { + fmt.Fprintln(stderr, err) + return exitFailure + } + markInitLockCreation(&plan) + report.Repair = &doctor.Repair{Finding: "template.identity_missing", Plan: plan} + if plan.Applied { + repaired, err := doctor.Run(doctor.Options{RepoRoot: repoRoot, ScopeRoot: scopeRoot, AgentFile: *agentFile, Profile: profile, MaxDepth: *maxDepth}) + if err != nil { + fmt.Fprintln(stderr, err) + return exitFailure + } + repaired.Repair = report.Repair + report = repaired + } + } if err := writeResult(stdout, *jsonOutput, report, func(writer io.Writer) { printDoctorReport(writer, report) }); err != nil { fmt.Fprintln(stderr, err) return exitFailure @@ -363,6 +408,33 @@ func runDoctor(arguments []string, stdout, stderr io.Writer) int { return exitSuccess } +func hasDoctorFinding(report doctor.Report, code string) bool { + for _, finding := range report.Findings { + if finding.Code == code { + return true + } + } + return false +} + +// markInitLockCreation gives the shared init plan its adoption-specific +// presentation: init writes a previously absent lock, rather than updating one. +func markInitLockCreation(plan *ownership.Report) { + for index := len(plan.Decisions) - 1; index >= 0; index-- { + if plan.Decisions[index].Path != ownership.LockFileName { + continue + } + plan.Decisions[index].Action = ownership.Create + plan.Decisions[index].Reason = "record successful adoption" + return + } + plan.Decisions = append(plan.Decisions, ownership.Decision{ + Path: ownership.LockFileName, + Action: ownership.Create, + Reason: "record successful adoption", + }) +} + func printDoctorReport(writer io.Writer, report doctor.Report) { fmt.Fprintf(writer, "Memory Bank doctor (%s profile)\n", report.Profile) if report.TemplateIdentity.Version != "" { @@ -376,6 +448,13 @@ func printDoctorReport(writer io.Writer, report doctor.Report) { fmt.Fprintf(writer, "%s\t%s\t%s\t%s\n", finding.Severity, finding.Code, subject, finding.Message) fmt.Fprintf(writer, " remediation: %s\n", finding.Remediation) } + if report.Repair != nil { + fmt.Fprintf(writer, "Repair plan for %s:\n", report.Repair.Finding) + printOwnershipReport(writer, report.Repair.Plan) + if report.Repair.Plan.Applied { + fmt.Fprintln(writer, "Repair completed: commit memory-bank/.lock before running update.") + } + } fmt.Fprintf(writer, "Result: %d error(s), %d warning(s), %d info\n", report.Summary.Errors, report.Summary.Warnings, report.Summary.Info) } diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index ed788c5..7dce3c8 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -428,6 +428,137 @@ func TestDoctorAndAlternativeAgentTarget(t *testing.T) { } } +func TestDoctorFixAdoptsMissingLockWithExplicitProvenance(t *testing.T) { + repo, source := t.TempDir(), t.TempDir() + readme := "---\ndoc_function: index\npurpose: Test index for doctor.\nstatus: active\n---\n# Memory Bank\n" + if err := os.MkdirAll(filepath.Join(source, "memory-bank"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(repo, "memory-bank"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(source, "memory-bank", "README.md"), []byte(readme), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(repo, "memory-bank", "README.md"), []byte(readme), 0o644); err != nil { + t.Fatal(err) + } + sourceRef := commitCLISource(t, source, "source") + base := []string{"doctor", "--fix", "--repo-root", repo, "--source", source, "--template-version", "v1", "--source-ref", sourceRef} + + var stdout, stderr bytes.Buffer + if exitCode := Run(append(append([]string{}, base...), "--dry-run", "--json"), "test", &stdout, &stderr); exitCode != 1 { + t.Fatalf("dry-run exit = %d, stderr=%s", exitCode, stderr.String()) + } + var dryRun struct { + Repair struct { + Finding string `json:"finding"` + Plan struct { + DryRun bool `json:"dry_run"` + Decisions []struct { + Path string `json:"path"` + } `json:"decisions"` + } `json:"plan"` + } `json:"repair"` + } + if err := json.Unmarshal(stdout.Bytes(), &dryRun); err != nil { + t.Fatalf("invalid dry-run report: %v\n%s", err, stdout.String()) + } + if !strings.Contains(stdout.String(), "memory-bank-cli doctor --fix") { + t.Fatalf("missing-lock remediation does not guide the repair: %s", stdout.String()) + } + if dryRun.Repair.Finding != "template.identity_missing" || !dryRun.Repair.Plan.DryRun || len(dryRun.Repair.Plan.Decisions) == 0 { + t.Fatalf("unexpected dry-run repair: %#v", dryRun) + } + lockPlanned := false + lockDecisionCount := 0 + for _, decision := range dryRun.Repair.Plan.Decisions { + if decision.Path == "memory-bank/.lock" { + lockPlanned = true + lockDecisionCount++ + } + } + if !lockPlanned { + t.Fatalf("dry-run omitted the lock creation: %#v", dryRun.Repair.Plan.Decisions) + } + if lockDecisionCount != 1 { + t.Fatalf("dry-run should contain one lock creation, got %d decisions: %#v", lockDecisionCount, dryRun.Repair.Plan.Decisions) + } + if _, err := os.Stat(filepath.Join(repo, "memory-bank", ".lock")); !os.IsNotExist(err) { + t.Fatalf("dry-run created a lock: %v", err) + } + + stdout.Reset() + stderr.Reset() + if exitCode := Run(base, "test", &stdout, &stderr); exitCode != 0 { + t.Fatalf("repair exit = %d, stderr=%s", exitCode, stderr.String()) + } + if !strings.Contains(stdout.String(), "commit memory-bank/.lock") { + t.Fatalf("repair did not recommend committing the lock: %s", stdout.String()) + } + if !strings.Contains(stdout.String(), "create\t\tmemory-bank/.lock\trecord successful adoption") { + t.Fatalf("repair report omitted the lock creation: %s", stdout.String()) + } + lock, err := os.ReadFile(filepath.Join(repo, "memory-bank", ".lock")) + if err != nil || !strings.Contains(string(lock), sourceRef) { + t.Fatalf("repair did not create the pinned lock: %q, %v", lock, err) + } + + stdout.Reset() + stderr.Reset() + if exitCode := Run([]string{"doctor", "--fix", "--repo-root", repo, "--json"}, "test", &stdout, &stderr); exitCode != 0 { + t.Fatalf("repair with an existing lock failed: exit=%d stderr=%s", exitCode, stderr.String()) + } + var existing struct { + Repair any `json:"repair"` + } + if err := json.Unmarshal(stdout.Bytes(), &existing); err != nil || existing.Repair != nil { + t.Fatalf("existing lock should not produce a repair: %#v, %v", existing, err) + } +} + +func TestDoctorFixRequiresProvenanceAndPreservesConflictingManagedContent(t *testing.T) { + repo, source := t.TempDir(), t.TempDir() + readme := "---\ndoc_function: index\npurpose: Test index for doctor.\nstatus: active\n---\n# Memory Bank\n" + if err := os.MkdirAll(filepath.Join(source, "template", "memory-bank"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(repo, "memory-bank"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(source, "template", "memory-bank", "README.md"), []byte(readme), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(repo, "memory-bank", "README.md"), []byte("local customization\n"), 0o644); err != nil { + t.Fatal(err) + } + sourceRef := commitCLISource(t, source, "source") + + var stdout, stderr bytes.Buffer + if exitCode := Run([]string{"doctor", "--fix", "--repo-root", repo}, "test", &stdout, &stderr); exitCode != exitUsage { + t.Fatalf("missing provenance exit = %d, stderr=%s", exitCode, stderr.String()) + } + if !strings.Contains(stderr.String(), "requires --source, --template-version, and --source-ref") { + t.Fatalf("missing provenance error is not actionable: %s", stderr.String()) + } + + stdout.Reset() + stderr.Reset() + arguments := []string{"doctor", "--fix", "--repo-root", repo, "--source", source, "--template-version", "v1", "--source-ref", sourceRef, "--json"} + if exitCode := Run(arguments, "test", &stdout, &stderr); exitCode != exitFailure { + t.Fatalf("conflicting repair exit = %d, stderr=%s", exitCode, stderr.String()) + } + if !strings.Contains(stdout.String(), "existing managed file does not match initialization source") { + t.Fatalf("conflicting managed content was not reported: %s", stdout.String()) + } + if data, err := os.ReadFile(filepath.Join(repo, "memory-bank", "README.md")); err != nil || string(data) != "local customization\n" { + t.Fatalf("conflicting managed content changed: %q, %v", data, err) + } + if _, err := os.Stat(filepath.Join(repo, "memory-bank", ".lock")); !os.IsNotExist(err) { + t.Fatalf("conflicting repair created a lock: %v", err) + } +} + func TestDoctorRejectsUnknownProfile(t *testing.T) { var stdout, stderr bytes.Buffer if exitCode := Run([]string{"doctor", "--profile", "mystery"}, "test", &stdout, &stderr); exitCode != 2 { diff --git a/internal/doctor/doctor.go b/internal/doctor/doctor.go index 4f24f68..843f44a 100644 --- a/internal/doctor/doctor.go +++ b/internal/doctor/doctor.go @@ -122,7 +122,7 @@ func (report *Report) checkIdentityAndDrift(agentFile, scopeRoot string) { 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."}) + 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: "Run memory-bank-cli doctor --fix with --source, --template-version, and --source-ref from a trusted template checkout, then 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."}) } diff --git a/internal/doctor/types.go b/internal/doctor/types.go index 56884d8..9a77515 100644 --- a/internal/doctor/types.go +++ b/internal/doctor/types.go @@ -1,12 +1,14 @@ // Package doctor diagnoses Memory Bank adoption and governance without mutation. package doctor -import "github.com/dapi/memory-bank-cli/internal/lint" +import ( + "github.com/dapi/memory-bank-cli/internal/lint" + "github.com/dapi/memory-bank-cli/internal/ownership" +) -// ReportFormatVersion identifies the aggregate doctor JSON schema. Version 2 -// reflects the replacement of the former ownership-style drift/conflict fields -// with the aggregate summary and findings contract. -const ReportFormatVersion = 2 +// ReportFormatVersion identifies the aggregate doctor JSON schema. Version 3 +// adds the optional structured repair plan emitted by doctor --fix. +const ReportFormatVersion = 3 type Profile string @@ -54,6 +56,14 @@ type Report struct { Summary Summary `json:"summary"` Findings []Finding `json:"findings"` Navigation lint.Report `json:"navigation"` + Repair *Repair `json:"repair,omitempty"` +} + +// Repair records the opt-in ownership operation performed for a finding. The +// plan is the same validated adoption plan produced by memory-bank-cli init. +type Repair struct { + Finding string `json:"finding"` + Plan ownership.Report `json:"plan"` } type Options struct {