diff --git a/CHANGELOG.md b/CHANGELOG.md index 22841aa..60b0eba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.ht ## [Unreleased] +### Changed + +- Let ordinary `pull` automatically apply Git-verified, deterministic + non-overlapping three-way merges for adapted files. Ambiguous, overlapping or + unverified changes still require a reviewed resolution plan. + ## [2.1.0] - 2026-08-15 ### Added diff --git a/README.md b/README.md index c457252..f59455f 100644 --- a/README.md +++ b/README.md @@ -40,8 +40,14 @@ is not a terminal. ## Prepare and apply a reviewed pull plan -When both an adapted downstream document and its upstream base changed, create -a complete reviewable plan instead of repeating the conservative pull: +Ordinary `pull` automatically applies a merge only when it can prove all of +the following: the historical Git source blob matches the base in `.lock`, both +files are regular supported files, and local and upstream line edits do not +overlap. It preserves both independent changes and records the new upstream +base atomically. + +Create a complete reviewable plan only when `pull` reports a remaining +conflict, or when you want an explicit audit record: ```sh memory-bank-cli pull --plan memory-bank-pull-plan.json @@ -67,8 +73,9 @@ memory-bank-cli pull --apply-plan memory-bank-pull-plan.json Apply resolves the current source again, strictly regenerates every non-decision field, and rejects unresolved, altered or stale input before mutation. Accepted resolutions, deterministic managed updates and `.lock` -commit atomically. Ordinary `pull` remains conservative and the CLI never -calls an LLM or treats a mechanical merge as semantic approval. +commit atomically. The CLI never calls an LLM or treats a mechanical merge as +semantic approval: overlapping edits, missing historical source and explicit +ownership choices still require review. Resolution plans may contain base64-encoded merged document content. Treat them with the same privacy as the downstream repository and review them before diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 936b0d4..e254f7f 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -234,7 +234,7 @@ func TestResolutionPlanFlagsArePublicAndMutuallyExclusive(t *testing.T) { } } -func TestResolutionPlanCLIReviewsCanonicalMigrationAndSecondPullIsNoOp(t *testing.T) { +func TestPullAutomaticallyMergesNonOverlappingCanonicalMigrationAndSecondPullIsNoOp(t *testing.T) { repo, source := t.TempDir(), t.TempDir() path := filepath.Join("memory-bank", "domain", "model.md") legacySourcePath := filepath.Join(source, path) @@ -284,30 +284,22 @@ func TestResolutionPlanCLIReviewsCanonicalMigrationAndSecondPullIsNoOp(t *testin if err := json.Unmarshal(mustReadFile(t, planPath), &plan); err != nil { t.Fatal(err) } - selected := false - for index := range plan.Entries { - if plan.Entries[index].Path == filepath.ToSlash(path) { - if plan.Entries[index].Merge == nil { - t.Fatalf("plan has no merge: %#v", plan.Entries[index]) + found := false + for _, entry := range plan.Entries { + if entry.Path == filepath.ToSlash(path) { + if entry.Merge == nil { + t.Fatalf("plan has no merge: %#v", entry) } - plan.Entries[index].SelectedAction = "apply-reviewed-merge" - selected = true + found = true } } - if !selected { + if !found { t.Fatalf("adapted conflict missing: %#v", plan.Entries) } - encoded, err := json.MarshalIndent(plan, "", " ") - if err != nil { - t.Fatal(err) - } - if err := os.WriteFile(planPath, append(encoded, '\n'), 0o600); err != nil { - t.Fatal(err) - } stdout.Reset() stderr.Reset() - if exitCode := Run(append(append([]string{}, pullArgs...), "--apply-plan", planPath), "test", &stdout, &stderr); exitCode != exitSuccess { - t.Fatalf("apply exit=%d stdout=%q stderr=%q", exitCode, stdout.String(), stderr.String()) + if exitCode := Run(pullArgs, "test", &stdout, &stderr); exitCode != exitSuccess { + t.Fatalf("automatic pull exit=%d stdout=%q stderr=%q", exitCode, stdout.String(), stderr.String()) } if got, want := string(mustReadFile(t, filepath.Join(repo, path))), "title\nupstream\nbase\nlocal\ntail\n"; got != want { t.Fatalf("merged=%q, want %q", got, want) diff --git a/internal/ownership/resolution_plan_test.go b/internal/ownership/resolution_plan_test.go index 3125a31..ef0fb96 100644 --- a/internal/ownership/resolution_plan_test.go +++ b/internal/ownership/resolution_plan_test.go @@ -239,6 +239,26 @@ func TestReviewedPlanDoesNotOfferUnrepresentableLocalDeletion(t *testing.T) { } } +func TestOrdinaryPullLeavesOverlappingAdaptedChangesForReview(t *testing.T) { + _, repo, options, path := resolutionConflictFixture(t) + beforeLock := read(t, repo, LockFileName) + beforeContent := read(t, repo, path) + + report, err := Update(options) + if err != nil { + t.Fatal(err) + } + if report.Applied || report.ConflictCount != 1 { + t.Fatalf("overlapping ordinary pull report=%#v", report) + } + if got := read(t, repo, path); got != beforeContent { + t.Fatalf("overlapping ordinary pull changed adapted file: %q", got) + } + if got := read(t, repo, LockFileName); got != beforeLock { + t.Fatal("overlapping ordinary pull changed lock") + } +} + func resolutionConflictFixture(t *testing.T) (string, string, Options, string) { t.Helper() source, repo := t.TempDir(), t.TempDir() diff --git a/internal/ownership/update.go b/internal/ownership/update.go index eed0861..72e7431 100644 --- a/internal/ownership/update.go +++ b/internal/ownership/update.go @@ -117,6 +117,17 @@ func run(options Options, old Lock, hasLock bool, repo pinnedRepo, lockDigest st if err := verifySource(pinnedSource.root, options.SourceRef); err != nil { return Report{}, fmt.Errorf("source checkout changed while reading template: %w", err) } + // An ordinary pull resolves only mechanically provable adapted-file merges: + // a locked historical base is available and the two line edits do not + // overlap. Everything else remains a conflict for --plan/--apply-plan (or + // another explicit resolution), so this never substitutes a semantic choice. + if hasLock && options.AdaptedResolutions == nil { + automatic, automaticErr := automaticAdaptedMergeResolutions(repo, pinnedSource, source, old, options) + if automaticErr != nil { + return Report{}, automaticErr + } + options.AdaptedResolutions = automatic + } mutations, decisions, next, err := buildPlan(repo, source, old, hasLock, options.UserOwnedResolutions, options.AdaptedResolutions, options.DetachUserOwnedRemovals) if err != nil { return Report{}, err @@ -202,6 +213,61 @@ func run(options Options, old Lock, hasLock bool, repo pinnedRepo, lockDigest st return report, nil } +// automaticAdaptedMergeResolutions finds only non-overlapping, Git-verified +// three-way merges. Missing history, unreadable paths, overlapping edits, and +// all non-adapted conflicts deliberately remain unresolved. +func automaticAdaptedMergeResolutions(repo pinnedRepo, currentSource pinnedSource, source map[string]payload, old Lock, options Options) (map[string]AdaptedResolution, error) { + _, decisions, _, err := buildPlan(repo, source, old, true, nil, nil, false) + if err != nil { + return nil, err + } + hasCandidate := false + for _, decision := range decisions { + if decision.Action == Conflict && decision.Ownership == Adapted { + hasCandidate = true + break + } + } + if !hasCandidate || options.verifySource != nil { + return nil, nil + } + + historical, err := readGitSource(currentSource, old.Template.SourceRef) + if err != nil { + // Ordinary pull remains usable when an old source object was pruned or + // otherwise cannot be verified; it will report the original conflict. + return nil, nil + } + resolutions := make(map[string]AdaptedResolution) + for _, decision := range decisions { + if decision.Action != Conflict || decision.Ownership != Adapted { + continue + } + prior, tracked := old.Files[decision.Path] + incoming, sourceExists := source[decision.Path] + base, baseExists := historical[decision.Path] + if !tracked || !sourceExists || !baseExists || base.digest != prior.BaseDigest || !modeMatches(base.mode, prior.BaseMode) { + continue + } + info, localData, localExists, readErr := readPlanDestination(repo, decision.Path) + if readErr != nil { + return nil, readErr + } + if !localExists { + continue + } + merged, mode, mergeErr := mechanicalMerge(base.data, localData, incoming.data, base.mode, observedMode(info.Mode().Perm()), incoming.mode) + if mergeErr != nil { + continue + } + resolutions[decision.Path] = AdaptedResolution{Action: "apply-automatic-merge", Data: merged, Mode: mode} + } + if len(resolutions) == 0 { + return nil, nil + } + return resolutions, nil +} + func buildAgentPlan(repo pinnedRepo, target string) (*mutation, Decision, error) { return buildAgentPlanWithReader(repo, target, secureReadDestination) } @@ -639,11 +705,15 @@ func buildPlan(repo pinnedRepo, source map[string]payload, old Lock, hasLock boo if class == Managed { file = File{Ownership: Managed, BaseDigest: incoming.digest, PayloadDigest: incoming.digest, BaseMode: incoming.mode, PayloadMode: incoming.mode} } - case "apply-reviewed-merge": + case "apply-reviewed-merge", "apply-automatic-merge": if resolution.Mode != "100644" && resolution.Mode != "100755" { return nil, nil, Lock{}, fmt.Errorf("invalid reviewed merge resolution for %s", path) } - decision.Action, decision.Reason = UpdateFile, "apply reviewed merge for adapted file" + if resolution.Action == "apply-automatic-merge" { + decision.Action, decision.Reason = UpdateFile, "automatically merge non-overlapping adapted changes" + } else { + decision.Action, decision.Reason = UpdateFile, "apply reviewed merge for adapted file" + } mutationData, mutationMode = resolution.Data, fileMode(resolution.Mode) default: return nil, nil, Lock{}, fmt.Errorf("invalid adapted resolution %q for %s", resolution.Action, path) diff --git a/memory-bank/features/FT-054-ai-assisted-human-approved-pull-resolution/brief.md b/memory-bank/features/FT-054-ai-assisted-human-approved-pull-resolution/brief.md index eeac234..146f02f 100644 --- a/memory-bank/features/FT-054-ai-assisted-human-approved-pull-resolution/brief.md +++ b/memory-bank/features/FT-054-ai-assisted-human-approved-pull-resolution/brief.md @@ -24,10 +24,11 @@ reviewable merge. Re-running `pull` repeats the same unapplied plan. ### Outcome -`pull` can emit a complete, non-mutating resolution plan. An agent may prepare -candidate decisions and mechanical merge results, a human reviews the plan, -and the CLI applies the reviewed result only while every recorded input still -matches. The default `pull` remains conservative. +`pull` automatically applies a verified deterministic merge when the locked +Git base is available and local/upstream line edits do not overlap. A complete, +non-mutating resolution plan remains available for ambiguous cases and audit: +an agent may prepare candidate decisions, a human reviews the plan, and the CLI +applies the reviewed result only while every recorded input still matches. ### Scope @@ -46,8 +47,9 @@ matches. The default `pull` remains conservative. stale, malformed or altered plan before mutation. - `REQ-05` Apply the selected resolutions, all deterministic safe changes and the new `.lock` through the existing atomic ownership transaction. -- `REQ-06` Keep ordinary `pull` and `pull --ask` backward compatible; the CLI - does not call an LLM or infer a semantic document decision. +- `REQ-06` Keep ordinary `pull` and `pull --ask` backward compatible. Ordinary + `pull` auto-applies only a Git-verified, deterministic non-overlapping line + merge; the CLI does not call an LLM or infer a semantic document decision. - `REQ-07` Keep user-owned content that disappeared upstream and detach its obsolete lock entry during reviewed full-plan apply. - `REQ-08` Document the trusted-local review model, commands, plan editing @@ -115,10 +117,11 @@ former `BD-01` and `BD-02` without a sidecar or protected registry. affected path and required human decision. - `EC-02` A complete reviewed plan applies atomically only against its exact recorded source, lock and local state. -- `EC-03` Every two-sided adapted conflict remains unresolved until an explicit - currently allowed action is selected. -- `EC-04` A verified non-overlapping merge writes exactly the reviewed bytes and - mode; unavailable or overlapping history cannot select merge. +- `EC-03` Every two-sided adapted conflict without a verified non-overlapping + merge remains unresolved until an explicit currently allowed action is + selected. +- `EC-04` Ordinary `pull` writes a verified non-overlapping merge atomically; + unavailable or overlapping history cannot auto-merge. - `EC-05` Stale, malformed or altered plans and injected transaction failures leave payload and lock unchanged. - `EC-06` A successful full-plan apply advances `.lock`; the next ordinary @@ -136,9 +139,9 @@ former `BD-01` and `BD-02` without a sidecar or protected registry. the adapted base, preventing the same conflict on the next pull. - `SC-04` `take-upstream` writes upstream bytes/mode and adopts canonical managed ownership during legacy-to-canonical migration. -- `SC-05` `apply-reviewed-merge` is offered only when the old source blob - matches `.lock`; its exact deterministic result is embedded, reviewed and - recomputed at apply. +- `SC-05` Ordinary `pull` applies a deterministic merge only when the old + source blob matches `.lock` and the line edits do not overlap. The same exact + result is embedded for optional reviewed apply. - `SC-06` If historical Git data is missing, mismatched, non-textual, overlapping or mode-ambiguous, planning keeps non-merge choices available and marks reviewed merge unavailable. @@ -147,9 +150,8 @@ former `BD-01` and `BD-02` without a sidecar or protected registry. causes stale/tamper rejection with no partial mutation. - `SC-09` A user-owned file removed upstream stays untouched and its obsolete lock entry is detached in the same successful reviewed transaction. -- `SC-10` Kirasa's three current canonical-migration conflicts produce clean - merge candidates, apply with the managed updates, and leave a second pull at - no-op. +- `SC-10` Kirasa's three current canonical-migration conflicts automatically + merge with the managed updates and leave a second pull at no-op. ### Negative Coverage diff --git a/memory-bank/features/FT-054-ai-assisted-human-approved-pull-resolution/decision-log.md b/memory-bank/features/FT-054-ai-assisted-human-approved-pull-resolution/decision-log.md index 212131b..672c3c3 100644 --- a/memory-bank/features/FT-054-ai-assisted-human-approved-pull-resolution/decision-log.md +++ b/memory-bank/features/FT-054-ai-assisted-human-approved-pull-resolution/decision-log.md @@ -27,6 +27,7 @@ solution. This ledger records why those decisions were accepted. | `DEC-04` | accepted | Recover historical base from `.lock.template.source_ref` and verify the Git blob against `BaseDigest`/`BaseMode`. | The immutable source identity and base digest already exist; the upstream fetch contains reachable history. | No lock-history sidecar or protected receipt registry. Missing/mismatched history disables merge only. | | `DEC-05` | accepted | Use deterministic non-overlapping line merge and include its exact result in the reviewed plan. | This allows mechanical assistance without claiming semantic correctness. | Apply recomputes exact bytes/mode; overlap keeps merge unavailable. | | `DEC-06` | accepted | Route issue #54 through the final implementation and release carrier. | PR #56 passed all required checks, merged as `b9ec3a2`, and closed issue #54. | Release `v2.1.0` carries the versioned changelog and installable binaries. | +| `DEC-07` | accepted | Let ordinary `pull` apply only Git-verified non-overlapping adapted-file merges. | The user requires a simple pull workflow that preserves independent local changes; this condition is deterministic but does not claim semantic understanding. | `--plan` is reserved for overlap, unavailable history, user-owned choices and audit. | ## Open Questions