diff --git a/src/MeshWeaver.Graph/StaticRepoImporter.cs b/src/MeshWeaver.Graph/StaticRepoImporter.cs index 5e455b5c2c..5e8ce66223 100644 --- a/src/MeshWeaver.Graph/StaticRepoImporter.cs +++ b/src/MeshWeaver.Graph/StaticRepoImporter.cs @@ -17,8 +17,10 @@ namespace MeshWeaver.Graph; /// Outcome of a run. is /// the number of live nodes a two-way import kept because they were newer on the server (see -/// ) — 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. +/// ) — 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. public sealed record StaticRepoImportResult(string Partition, string Fingerprint, string Outcome, int Count = 0, int Preserved = 0); /// @@ -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(); 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); }); }))); }); diff --git a/test/MeshWeaver.GitSync.Test/TwoWaySyncTest.cs b/test/MeshWeaver.GitSync.Test/TwoWaySyncTest.cs index 4df6e655ad..ca77528078 100644 --- a/test/MeshWeaver.GitSync.Test/TwoWaySyncTest.cs +++ b/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")); } + + /// + /// 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. + /// + [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")); + } + + /// Simulates a repo-side commit: the current tree plus one added file. + private Task 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(); }