fix(client): three-way merge when a file changed on both sides

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

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
 	}
 

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")