GitSync: a server-side addition was silently deleted one sync cycle later

src/MeshWeaver.Graph/StaticRepoImporter.cs

@@ -17,8 +17,10 @@ namespace MeshWeaver.Graph;
 
 /// <summary>Outcome of a <see cref="StaticRepoImporter.Import"/> run. <paramref name="Preserved"/> is
 /// the number of live nodes a two-way import kept because they were newer on the server (see
-/// <see cref="ImportConflictPolicy"/>) — non-zero means the mesh is now AHEAD of the repo, so the
-/// caller must NOT advance its last-sync baseline until those edits are committed back.</summary>
+/// <see cref="ImportConflictPolicy"/>) — BOTH kinds: kept-not-overwritten (the repo's copy differs) and
+/// kept-not-pruned (the repo has no copy at all — a server-side addition). Non-zero means the mesh is
+/// now AHEAD of the repo, so the caller must NOT advance its last-sync baseline until those changes are
+/// committed back.</summary>
 public sealed record StaticRepoImportResult(string Partition, string Fingerprint, string Outcome, int Count = 0, int Preserved = 0);
 
 /// <summary>
@@ -858,8 +860,27 @@ bool IsClaimed(string path, MeshNode? target) =>
                     // Two-way: never prune a node CREATED/changed on the server since the last sync. It
                     // isn't in the repo yet, but it's a local addition to be committed back — not a stale
                     // extra to remove. A forced import prunes it (the repo state wins).
+                    // 🚨 A node KEPT here counts as PRESERVED exactly like a kept upsert conflict — it is
+                    // the same statement: "this local change is not in the repo yet, so the mesh is AHEAD".
+                    // Reporting 0 here let GitHubSyncService.ReimportAtCommit advance the sync baseline
+                    // (LastSyncedAt) past the node it had just protected, so on the NEXT import the node was
+                    // no longer "newer on the server" and got PRUNED — the server addition silently deleted
+                    // one cycle later, which is precisely what that baseline guard exists to prevent.
+                    // Deterministic repro: TwoWaySyncTest.TwoWay_ServerAddition_SurvivesEveryLaterRepoPush.
+                    var keptFromPrune = Array.Empty<MeshNode>();
                     if (policy is { PreserveServerNewer: true, Force: false, Since: { } pruneSince })
+                    {
+                        keptFromPrune = toPrune.Where(n => n.LastModified > pruneSince).ToArray();
                         toPrune = toPrune.Where(n => n.LastModified <= pruneSince).ToArray();
+                        foreach (var kept in keptFromPrune)
+                        {
+                            logger?.LogInformation(
+                                "[StaticRepoImport] {Partition}: two-way — keeping server-added {Path} (not pruned).",
+                                source.Partition, kept.Path);
+                            NodeTypeCompilationActivity.AppendLog(hub, activityPath,
+                                $"↩ Kept {kept.Path} (added on the server — commit to sync it back).", logger!);
+                        }
+                    }
 
                     var pruned = toPrune.Count == 0
                         ? Observable.Return(0)
@@ -900,8 +921,10 @@ bool IsClaimed(string path, MeshNode? target) =>
                             // full diagnostic log (no torn "Succeeded but the ⚠ lines didn't land yet").
                             var failed = count.Failed;
                             var status = failed > 0 ? ActivityStatus.Warning : ActivityStatus.Succeeded;
-                            // Two-way preserved local edits (kept, not overwritten) noted only when any.
-                            var preservedNote = count.Preserved > 0 ? $", kept {count.Preserved} local edit(s)" : "";
+                            // Two-way preserved local changes: kept-not-overwritten (upsert conflicts) PLUS
+                            // kept-not-pruned (server additions). Both leave the mesh ahead of the repo.
+                            var preserved = count.Preserved + keptFromPrune.Length;
+                            var preservedNote = preserved > 0 ? $", kept {preserved} local change(s)" : "";
                             var summary = failed > 0
                                 ? $"Imported {count.Imported} node(s), {failed} FAILED (see ⚠ above){preservedNote}, pruned {prunedCount}, synced {contentCount} content file(s)."
                                 : $"Imported {count.Imported} node(s){preservedNote}, pruned {prunedCount}, synced {contentCount} content file(s).";
@@ -918,7 +941,7 @@ bool IsClaimed(string path, MeshNode? target) =>
                                 "[StaticRepoImport] {Partition}: imported {Count}, failed {Failed}, pruned {Pruned}, content {Content} at {Fingerprint}.",
                                 source.Partition, count.Imported, failed, prunedCount, contentCount, fingerprint);
                             return new StaticRepoImportResult(source.Partition, fingerprint,
-                                failed > 0 ? "ImportedWithErrors" : "Imported", count.Imported, count.Preserved);
+                                failed > 0 ? "ImportedWithErrors" : "Imported", count.Imported, preserved);
                         });
                         })));
                 });

test/MeshWeaver.GitSync.Test/TwoWaySyncTest.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Immutable;
 using System.Reactive.Linq;
 using System.Reactive.Threading.Tasks;
 using MeshWeaver.GitSync;
@@ -55,4 +56,67 @@ await Sync.SaveConfig(a, repo, "main", subdirectory: null,
         Assert.True(await IsAbsent($"{a}/ServerOnly"));
         Assert.NotNull(await WaitForNode($"{a}/Welcome"));
     }
+
+    /// <summary>
+    /// The same baseline invariant under REAL repo traffic — the deterministic form of the guard
+    /// above (which relies on the second update short-circuiting on an unchanged fingerprint).
+    /// Every update here imports a genuinely new commit, so prune runs for real each time: a node
+    /// added on the server must survive update after update until it is committed back, because
+    /// PRESERVING it (here: by not pruning it) leaves the mesh AHEAD of the repo and must NOT
+    /// advance the sync baseline. Advancing it loses the node on the NEXT push — silent data loss.
+    /// </summary>
+    [Fact(Timeout = 120000)]
+    public async Task TwoWay_ServerAddition_SurvivesEveryLaterRepoPush()
+    {
+        await Connect();
+        var a = "GhY" + Guid.NewGuid().ToString("N")[..8];
+        await CreateSpace(a, "Space Y");
+        await CreateMarkdown($"{a}/Welcome", "Welcome", "# Welcome\n\nv1.");
+
+        var repo = "https://github.com/test/space-y";
+        await Sync.SaveConfig(a, repo, "main", subdirectory: null,
+                createBranchIfMissing: true, createRepoIfMissing: true,
+                direction: SyncDirection.Bidirectional, sourceId: null, twoWay: true)
+            .Timeout(30.Seconds()).ToTask();
+
+        var c1 = await Sync.SyncToGitHub(a, UserId).Timeout(60.Seconds()).ToTask();
+        var baseline = await WaitForConfig(a, c => c.LastSyncedAt != null && c.LastSyncCommitSha == c1.CommitSha);
+
+        // A user adds a node on the SERVER after that baseline — not in the repo at any commit.
+        await CreateMarkdown($"{a}/ServerOnly", "ServerOnly", "# ServerOnly\n\nadded on the server.");
+
+        // Someone commits to the repo; the update imports it and KEEPS the server addition.
+        await PushRepoFile(repo, "Docs.md", "# Docs\n\nfrom the repo.");
+        var update1 = await Sync.ReimportAtCommit(a, "main", UserId).Timeout(90.Seconds()).ToTask();
+        Assert.NotNull(await WaitForNode($"{a}/Docs"));
+        Assert.NotNull(await WaitForNode($"{a}/ServerOnly"));
+
+        // The invariant the survival above RESTS on, asserted directly: keeping the server addition
+        // (by not pruning it) is a PRESERVE, and a preserving import leaves the mesh ahead of the
+        // repo — so the sync baseline must NOT advance past the node it just protected. Reporting
+        // Preserved=0 here is what silently disarmed the protection and deleted the node one update
+        // later; the assertions below would then fail.
+        Assert.True(update1.Preserved > 0, $"kept-not-pruned must count as preserved (was {update1.Preserved})");
+        var afterUpdate1 = await Sync.ReadConfig(a).Timeout(30.Seconds()).ToTask();
+        Assert.Equal(baseline.LastSyncedAt, afterUpdate1!.LastSyncedAt);
+
+        // A SECOND repo commit must not delete it either — the baseline may not have moved past it.
+        await PushRepoFile(repo, "Docs2.md", "# Docs2\n\nfrom the repo.");
+        await Sync.ReimportAtCommit(a, "main", UserId).Timeout(90.Seconds()).ToTask();
+        Assert.NotNull(await WaitForNode($"{a}/Docs2"));
+        Assert.NotNull(await WaitForNode($"{a}/ServerOnly"));
+    }
+
+    /// <summary>Simulates a repo-side commit: the current tree plus one added file.</summary>
+    private Task<GitHubPushResult> PushRepoFile(string repo, string path, string content) =>
+        Fake.Push(new GitHubPushRequest
+        {
+            RepositoryUrl = repo,
+            Branch = "main",
+            Files = Fake.Tree(repo).Append(new RepoFile(path, content)).ToImmutableList(),
+            CommitMessage = $"add {path}",
+            AuthorName = "octocat",
+            AuthorEmail = "octocat@users.noreply.github.com",
+            AccessToken = "ghp_test_token",
+        }).Timeout(30.Seconds()).ToTask();
 }