From 30cd772f08ca7eaee91e3b70b6a5c8eaee4f1db0 Mon Sep 17 00:00:00 2001 From: h Date: Fri, 2 Oct 2026 23:31:16 +0000 Subject: [PATCH] fix(client): three-way merge when a file changed on both sides --- backend/.golangci.yml | 1 + backend/internal/client/sync.go | 57 ++++++++++++++++++++++- backend/internal/client/sync_test.go | 67 ++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+), 2 deletions(-) diff --git a/backend/.golangci.yml b/backend/.golangci.yml index 6a08464..7a71d70 100644 --- a/backend/.golangci.yml +++ b/backend/.golangci.yml @@ -44,6 +44,7 @@ linters: - (*github.com/fsnotify/fsnotify.Watcher).Close - os.Unsetenv - os.Remove + - os.RemoveAll - fmt.Fprint - fmt.Fprintf - fmt.Fprintln diff --git a/backend/internal/client/sync.go b/backend/internal/client/sync.go index 7040845..22d863b 100644 --- a/backend/internal/client/sync.go +++ b/backend/internal/client/sync.go @@ -1,6 +1,7 @@ package client import ( + "bytes" "cmp" "context" "encoding/json" @@ -9,10 +10,12 @@ import ( "maps" "net/http" "os" + "os/exec" "path/filepath" "slices" "strings" "syscall" + "unicode/utf8" "lodge/internal/gitrepo" "lodge/internal/protocol" @@ -176,7 +179,9 @@ func (c *Client) scan(repo *gitrepo.Repo, rules Rules) diskFiles { return found } -func reconcile(space string, local, base, remote protocol.Manifest, relevant func(string) bool) (first, second []protocol.Change, pull, next protocol.Manifest) { +type mergeFunc func(local, base, remote protocol.File) (protocol.File, bool) + +func reconcile(space string, local, base, remote protocol.Manifest, relevant func(string) bool, merge mergeFunc) (first, second []protocol.Change, pull, next protocol.Manifest) { pull, next = protocol.Manifest{}, maps.Clone(base) if next == nil { next = protocol.Manifest{} @@ -192,6 +197,14 @@ func reconcile(space string, local, base, remote protocol.Manifest, relevant fun continue } l, b, r := local[path], base[path], remote[path] + if l.Hash != b.Hash && r.Hash != b.Hash && l.Hash != r.Hash { + if merged, ok := merge(l, b, r); ok { + if merged.Hash != l.Hash { + pull[path] = merged + } + l, b = merged, r + } + } mine := protocol.Change{Space: space, Path: path, Base: r.Hash, Hash: l.Hash} settled := r switch { @@ -215,6 +228,43 @@ func reconcile(space string, local, base, remote protocol.Manifest, relevant fun return first, second, pull, next } +func (c *Client) merge(ctx context.Context, local, base, remote protocol.File) (protocol.File, bool) { + names := []string{"local", "base", "remote"} + versions := make([][]byte, len(names)) + for i, file := range []protocol.File{local, base, remote} { + if file.Hash == "" { + return protocol.File{}, false + } + data, err := c.Blob(ctx, file.Hash) + if err != nil || !isText(data) { + return protocol.File{}, false + } + versions[i] = data + } + dir, err := os.MkdirTemp(c.DataDir, "merge-") + if err != nil { + return protocol.File{}, false + } + defer os.RemoveAll(dir) + for i, name := range names { + if err := os.WriteFile(filepath.Join(dir, name), versions[i], 0o600); err != nil { + return protocol.File{}, false + } + } + cmd := exec.CommandContext(ctx, "git", append([]string{"merge-file", "-p"}, names...)...) + cmd.Dir = dir + merged, err := cmd.Output() + if err != nil || len(merged) > protocol.MaxBlobSize { + return protocol.File{}, false + } + hash, err := c.cacheBlob(merged) + return protocol.File{Hash: hash, Time: max(local.Time, remote.Time)}, err == nil +} + +func isText(data []byte) bool { + return len(data) <= protocol.MaxBlobSize && utf8.Valid(data) && bytes.IndexByte(data, 0) == -1 +} + func (c *Client) pass(ctx context.Context, repo *gitrepo.Repo, st *worktreeState, online bool, res *Result) (bool, error) { rules := LoadRules(repo.Root) if err := writeExclude(repo, rules); err != nil { @@ -261,6 +311,9 @@ func (c *Client) pass(ctx context.Context, repo *gitrepo.Repo, st *worktreeState var first, second []protocol.Change pulls, bases := map[string]protocol.Manifest{}, map[string]protocol.Manifest{} spaces := map[string]bool{} + merge := func(l, b, r protocol.File) (protocol.File, bool) { + return c.merge(ctx, l, b, r) + } for _, group := range []map[string]protocol.Manifest{locals, st.Base, remote.Spaces} { for space := range group { spaces[space] = true @@ -283,7 +336,7 @@ func (c *Client) pass(ctx context.Context, repo *gitrepo.Repo, st *worktreeState return protocol.SafePath(path) && rules.covers(path, space == protocol.SharedSpace) && !found.tracked[path] } } - mine, theirs, pull, next := reconcile(space, local, st.Base[space], remote.Spaces[space], relevant) + mine, theirs, pull, next := reconcile(space, local, st.Base[space], remote.Spaces[space], relevant, merge) first, second, pulls[space], bases[space] = append(first, mine...), append(second, theirs...), pull, next } diff --git a/backend/internal/client/sync_test.go b/backend/internal/client/sync_test.go index 7de02b7..00d5b6a 100644 --- a/backend/internal/client/sync_test.go +++ b/backend/internal/client/sync_test.go @@ -203,6 +203,73 @@ func TestLaterEditWinsAndLoserStaysInHistory(t *testing.T) { } } +func TestConcurrentEditsOfDifferentLinesMerge(t *testing.T) { + mac, dell := setup(t) + path := "docs/agents/plan.md" + write(t, mac.root, path, "# plan\n\n## todo\n- a\n\n## notes\n- x\n") + mac.sync() + dell.sync() + + write(t, mac.root, path, "# plan\n\n## todo\n- a\n- from mac\n\n## notes\n- x\n") + write(t, dell.root, path, "# plan\n\n## todo\n- a\n\n## notes\n- x\n- from dell\n") + dell.sync() + mac.sync() + dell.sync() + + merged := "# plan\n\n## todo\n- a\n- from mac\n\n## notes\n- x\n- from dell\n" + mac.expect(path, merged) + dell.expect(path, merged) + state, err := mac.client.State(context.Background(), repoKey(t, mac), "") + if err != nil { + t.Fatal(err) + } + if got, _ := mac.client.Blob(context.Background(), state.Spaces[protocol.SharedSpace][path].Hash); string(got) != merged { + t.Fatalf("server has %q, want %q", got, merged) + } + for _, m := range []*machine{mac, dell, mac} { + if res := m.sync(); res.Pushed != 0 || res.Pulled != 0 { + t.Fatalf("merged file keeps syncing: %+v", res) + } + } + revisions, err := mac.client.Revisions(context.Background(), repoKey(t, mac), path) + if err != nil || len(revisions) != 3 { + t.Fatalf("want base, dell and one merged revision, got %d (%v)", len(revisions), err) + } +} + +func TestConflictingOrBinaryEditsFallBackToLaterWins(t *testing.T) { + for name, versions := range map[string][3]string{ + "same line": {"1\n2\n3\n4\n5\n", "mac\n2\n3\n4\n5\n", "dell\n2\n3\n4\n5\n"}, + "nul byte": {"1\n2\n3\n4\n5\n\x00\n", "mac\n2\n3\n4\n5\n\x00\n", "1\n2\n3\n4\ndell\n\x00\n"}, + "invalid utf8": {"1\n2\n3\n4\n5\n\xff\n", "mac\n2\n3\n4\n5\n\xff\n", "1\n2\n3\n4\ndell\n\xff\n"}, + } { + t.Run(name, func(t *testing.T) { + mac, dell := setup(t) + path := "docs/agents/plan.md" + write(t, mac.root, path, versions[0]) + mac.sync() + dell.sync() + + write(t, mac.root, path, versions[1]) + old := time.Now().Add(-time.Hour) + if err := os.Chtimes(filepath.Join(mac.root, path), old, old); err != nil { + t.Fatal(err) + } + write(t, dell.root, path, versions[2]) + dell.sync() + mac.sync() + dell.sync() + + mac.expect(path, versions[2]) + dell.expect(path, versions[2]) + revisions, err := mac.client.Revisions(context.Background(), repoKey(t, mac), path) + if err != nil || len(revisions) != 4 { + t.Fatalf("want base, dell, losing mac and restored dell revisions, got %d (%v)", len(revisions), err) + } + }) + } +} + func TestRulesFileTravelsAndHostileServerPathsAreIgnored(t *testing.T) { mac, dell := setup(t) write(t, mac.root, RulesFile, "shared notes\n")