From 331c1f5b6f41d70fbffcfd22baf8ee0226194484 Mon Sep 17 00:00:00 2001 From: h Date: Fri, 2 Oct 2026 23:32:44 +0000 Subject: [PATCH] fix(client): merge concurrent appends at end of file --- backend/internal/client/sync.go | 17 ++++++- backend/internal/client/sync_test.go | 71 ++++++++++++++++------------ 2 files changed, 58 insertions(+), 30 deletions(-) diff --git a/backend/internal/client/sync.go b/backend/internal/client/sync.go index 22d863b..e7460bd 100644 --- a/backend/internal/client/sync.go +++ b/backend/internal/client/sync.go @@ -254,13 +254,28 @@ func (c *Client) merge(ctx context.Context, local, base, remote protocol.File) ( 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 { + clean := err == nil + if !clean { + merged, clean = appended(versions[0], versions[1], versions[2]) + } + if !clean || 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 appended(local, base, remote []byte) ([]byte, bool) { + if !bytes.HasPrefix(local, base) || !bytes.HasPrefix(remote, base) { + return nil, false + } + tail, glue := local[len(base):], []byte{} + if !bytes.HasSuffix(remote, []byte("\n")) && !bytes.HasPrefix(tail, []byte("\n")) { + glue = []byte("\n") + } + return slices.Concat(remote, glue, tail), true +} + func isText(data []byte) bool { return len(data) <= protocol.MaxBlobSize && utf8.Valid(data) && bytes.IndexByte(data, 0) == -1 } diff --git a/backend/internal/client/sync_test.go b/backend/internal/client/sync_test.go index 00d5b6a..7a8c5d2 100644 --- a/backend/internal/client/sync_test.go +++ b/backend/internal/client/sync_test.go @@ -203,37 +203,50 @@ 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() +func TestConcurrentEditsMerge(t *testing.T) { + for name, versions := range map[string][4]string{ + "different lines": { + "# plan\n\n## todo\n- a\n\n## notes\n- x\n", + "# plan\n\n## todo\n- a\n- from mac\n\n## notes\n- x\n", + "# plan\n\n## todo\n- a\n\n## notes\n- x\n- from dell\n", + "# plan\n\n## todo\n- a\n- from mac\n\n## notes\n- x\n- from dell\n", + }, + "appends": {"# log\n- a\n", "# log\n- a\n- from mac\n", "# log\n- a\n- from dell\n", "# log\n- a\n- from dell\n- from mac\n"}, + "appends without newline": {"# log\n- a", "# log\n- a\n- from mac", "# log\n- a\n- from dell", "# log\n- a\n- from dell\n- from mac"}, + "appends to unfinished line": {"# log\n- a", "# log\n- a b", "# log\n- a\n- from dell", "# log\n- a\n- from dell\n b"}, + } { + 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, "# 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() + write(t, mac.root, path, versions[1]) + write(t, dell.root, path, versions[2]) + 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) + mac.expect(path, versions[3]) + dell.expect(path, versions[3]) + 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) != versions[3] { + t.Fatalf("server has %q, want %q", got, versions[3]) + } + 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) + } + }) } }